Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[15.0] auth_saml_environment: removes readonly attributes to allow provider creation #177

Closed
wants to merge 1 commit into from

Conversation

StephaneMangin
Copy link

@StephaneMangin StephaneMangin commented Feb 14, 2024

Some readonly fields prevent the creation of a provider. Removing them as they will be managed by the env.

Following change in OCA/server-auth#342

(edit @yvaucher adding link to the source of the issue)

@@ -8,6 +8,11 @@ class AuthSamlProvider(models.Model):
_name = "auth.saml.provider"
_inherit = ["auth.saml.provider", "server.env.mixin"]

# Mandatory to be able to create objects
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Mandatory to be able to create objects
# Non-mandatory to be able to create objects

@yvaucher
Copy link
Member

yvaucher commented Mar 18, 2024

You don't want to state the odoo version in the commit message but rather the type of change. The odoo version is to be mentionned in the title of the PR.

Can you replace the content of the commit message by:

    [FIX] auth_saml_environment: no require on readonly fields
    
    Restore the usage of the form view by dropping the required condition on
    binary fields that are readonly.
    
    Those fields became mandatory after https://github.com/OCA/server-auth/pull/342
    but the module auth_saml_environment offer to use paths as a alternative config
    option for binaries thus those fields can be empty.

@yvaucher
Copy link
Member

Backported here #179

@vincent-hatakeyama
Copy link

vincent-hatakeyama commented Mar 18, 2024

idp_metadata is required for a provider to work. In a similar way, the server hostname of an outgoing server is mandatory. mail_environment does not change smtp_host to be optional, so I do not see why this module would do so for idp_metadata.

For sp_pem_public and and sp_pem_private, I would recommend fixing issue OCA/server-auth/issues/315 in auth_saml that made me change them to mandatory rather than adding another level of work around.

Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Jul 21, 2024
@github-actions github-actions bot closed this Aug 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants