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

Authorization refactoring #263

Closed
wants to merge 12 commits into from

Conversation

daines
Copy link
Contributor

@daines daines commented Mar 6, 2014

Addresses https://www.pivotaltracker.com/story/show/66946122

Rename FedoraPolicyEnforcementPoint interface to FedoraAuthorizationDelegate.
Instead of passing principals to PEP/authorization delegates, pass the session instance.
User and group principals added as session attributes.
ServletContainerAuthenticationProvider adds the servlet request as a session attribute (so authorization classes can access request IP address, etc).
Changed FedoraUserSecurityContext constructor to take a Principal instance rather than ServletCredentials.
Added additional documentation for PEP/authz delegate and other methods.

Rename FedoraPolicyEnforcementPoint interface to FedoraAuthorizationDelegate.
Instead of passing principals to PEP/authorization delegates, pass the session instance.
User and group principals added as session attributes.
ServletContainerAuthenticationProvider adds the servlet request as a session attribute (so authorization classes can access request IP address, etc).
Changed FedoraUserSecurityContext constructor to take a Principal instance rather than ServletCredentials.
Added additional documentation for PEP/authz delegate and other methods.
}

private void addUserPrincipals(final HttpServletRequest request,
private void addPrincipals(final Credentials credentials,
Copy link

Choose a reason for hiding this comment

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

It seems cleaner to change #addPrincipals(credentials, principals) to:

private Set collectPrincipals(final Credentials credentials) {...}

@awoods
Copy link

awoods commented Mar 8, 2014

Resolved with: 3bf9967
Addresses: https://www.pivotaltracker.com/story/show/66946122

@awoods awoods closed this Mar 8, 2014
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.

None yet

4 participants