Skip to content

Specify policy configurations based on the registration authority #729

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

Conversation

johanlundberg
Copy link
Contributor

@johanlundberg johanlundberg commented Sep 30, 2020

Allow the use of registration authorities in policy configuration.
This should work in the same way as for the policies written for specific SPs.

Example configuration

"idp": {
    "policy": {
        "a-service-entity-id": {
            ...
        }
        "a-registration-authority-name": {
            "attribute_restrictions": {
                "givenName": None,
            },
        },
        "default": {
            ...
        }
    }
}

References

All Submissions:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you added an explanation of what problem you are trying to solve with this PR?
  • Have you added information on what your changes do and why you chose this as your solution?
  • Have you written new tests for your changes?
  • Does your submission pass tests?
  • This project follows PEP8 style guide. Have you run your code against the 'flake8' linter?

@c00kiemon5ter c00kiemon5ter force-pushed the feature_registry_authority_policy branch 4 times, most recently from 9b76d55 to 6b3236f Compare October 25, 2020 18:49
else:
self._restrictions = None
def __init__(self, restrictions=None, mds=None):
self.metadata_store = mds
Copy link
Member

Choose a reason for hiding this comment

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

Policy needs a metadata store to look up attributes (entity-categories, registration-authority, etc).

Previously filter, restrict, and get_entity_categories requested the metadata store (aka mds, mdstore) as a param. Now, the one that has been supplied during initialization is used.

Copy link
Member

Choose a reason for hiding this comment

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

There are many changes, but this is the core of it.

To make this possible, the config module changed to build a Policy object with the metadata store. The metadata store must have been loaded beforehand.

Once the Policy object got a metadata-store object, the interfaces were refactored to remove the explicit param. The tests where then adjusted.

if ra_restrictions is not None
else default_restrictions
if default_restrictions is not None
else {}
Copy link
Member

@c00kiemon5ter c00kiemon5ter Oct 25, 2020

Choose a reason for hiding this comment

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

Use the sp_restrictions, else the ra_restrictions, else the default_restrictions - there is no more a default for the registration authorities; SPs and RAs are mixed on the same list.

return self.get("entity_categories", sp_entity_id, default={},
post_func=post_entity_categories, **kwargs)
result1 = self.get("entity_categories", sp_entity_id, default={})
result2 = post_entity_categories(
Copy link
Member

Choose a reason for hiding this comment

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

this should be further refactored

@c00kiemon5ter c00kiemon5ter force-pushed the feature_registry_authority_policy branch from 6b3236f to 8ce1a08 Compare October 25, 2020 19:01
Copy link
Member

@c00kiemon5ter c00kiemon5ter left a comment

Choose a reason for hiding this comment

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

I think this is fine now and we should go ahead and merge 👍

@johanlundberg do have a look, and if you can test it, and let me know.

@c00kiemon5ter c00kiemon5ter changed the title Feature registry authority policy Specify policy configurations for registration authorities Oct 25, 2020
@c00kiemon5ter c00kiemon5ter changed the title Specify policy configurations for registration authorities Specify policy configurations based on the registration authority Oct 25, 2020
@c00kiemon5ter c00kiemon5ter force-pushed the feature_registry_authority_policy branch from 8ce1a08 to 60600a2 Compare October 25, 2020 20:56
@c00kiemon5ter
Copy link
Member

c00kiemon5ter commented Oct 25, 2020

There are some breaking changes in this PR

  • saml2.config.Config.load does not accept the metadata_construction param
  • saml2.config.Config.load_file does not accept the metadata_construction param
  • saml2.assertion.Policy.restrict does not accept the metadata param
  • saml2.assertion.Policy.filter does not accept the mdstore param

@c00kiemon5ter c00kiemon5ter force-pushed the feature_registry_authority_policy branch from 4efb0ed to 618a67b Compare October 25, 2020 22:06
@johanlundberg
Copy link
Contributor Author

@c00kiemon5ter I approve :) If I am able I will try it in our staging environment but don't let that stop you from merging this to master if I am slow.

@johanlundberg
Copy link
Contributor Author

This feature is now running in our staging environment and fixes the attribute release problem we had when trying to be compliant with https://release-check.swamid.se/.

johanlundberg and others added 10 commits October 30, 2020 12:55
Signed-off-by: Ivan Kanakarakis <ivan.kanak@gmail.com>
Signed-off-by: Ivan Kanakarakis <ivan.kanak@gmail.com>
Signed-off-by: Ivan Kanakarakis <ivan.kanak@gmail.com>
Signed-off-by: Ivan Kanakarakis <ivan.kanak@gmail.com>
Signed-off-by: Ivan Kanakarakis <ivan.kanak@gmail.com>
… options

Signed-off-by: Ivan Kanakarakis <ivan.kanak@gmail.com>
@c00kiemon5ter c00kiemon5ter force-pushed the feature_registry_authority_policy branch from 618a67b to e9aed0f Compare October 30, 2020 11:02
Signed-off-by: Ivan Kanakarakis <ivan.kanak@gmail.com>
Signed-off-by: Ivan Kanakarakis <ivan.kanak@gmail.com>
Signed-off-by: Ivan Kanakarakis <ivan.kanak@gmail.com>
Signed-off-by: Ivan Kanakarakis <ivan.kanak@gmail.com>
@c00kiemon5ter c00kiemon5ter force-pushed the feature_registry_authority_policy branch from e9aed0f to 2641019 Compare October 30, 2020 15:24
@c00kiemon5ter
Copy link
Member

c00kiemon5ter commented Oct 30, 2020

There are some breaking changes in this PR

  • saml2.config.Config.load does not accept the metadata_construction param
  • saml2.config.Config.load_file does not accept the metadata_construction param
  • saml2.assertion.Policy.restrict does not accept the metadata param
  • saml2.assertion.Policy.filter does not accept the mdstore param

Instead of breaking the interfaces, warnings are generated. These will stay there for some time, until we remove them entirely.

This changeset is now backwards compatible, but to get the new features (restrictions based on the registration authority) one needs to properly upgrade, by initializing the Policy object with a metadata store. Usage that involves loading the configuration through the saml2.config.Config object get this automatically (this includes, the saml2.server.Server (IdP) object and the saml2.client_base.Base and saml2.client.Saml2Client (SP) objects.)

@c00kiemon5ter c00kiemon5ter merged commit 59d6fa5 into IdentityPython:master Oct 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants