Skip to content

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

Closed
StephaneMangin wants to merge 1 commit intoOCA:15.0from
camptocamp:allow_provider_creation
Closed

[15.0] auth_saml_environment: removes readonly attributes to allow provider creation#177
StephaneMangin wants to merge 1 commit intoOCA:15.0from
camptocamp:allow_provider_creation

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)

_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
Contributor

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.

@github-actions
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