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

[WiP] Separate authorization models + add Crowd #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fbelzunc
Copy link

@fbelzunc fbelzunc commented Feb 5, 2019

This is a Work in Progress

This is a PR which intends two different things:

  1. Separate the authorization models from the SecurityRealm
  2. Add the possibility to use crowd (right now only groups by header and ldap works)
  • Work progress

[ X ] Separate authorization models from the SecurityRealm
[ X ] Add crowd authorization model
[ X ] Correct findbugs
[ X ] Add readResolve to migrate the data
[ X ] Create new tests with readResolve
[ X ] More code refractor

  • Notes:
  • The implementation of the cache should use a different approach, but it is not my intention to fix all the plugin issues on this PR.

I am waiting for the review of the maintainer.

CC @Wadeck

@fbelzunc fbelzunc changed the title [WiP] Separate authentication models + add Crowd [WiP] Separate authorization models + add Crowd Feb 5, 2019
@fbelzunc fbelzunc force-pushed the separate-authentication branch from 29b233d to 134559e Compare February 6, 2019 11:47
@jglick
Copy link
Member

jglick commented Aug 27, 2020

Note that the usage of “authorization” in this plugin is nonstandard, as it is not defining an AuthorizationStrategy. Rather, this usage is a piece of authentication: looking up (e.g., LDAP) group names based on a user name.

@jglick
Copy link
Member

jglick commented Oct 7, 2020

Would clash more or less superficially with #40.

Copy link
Member

@jtnord jtnord left a comment

Choose a reason for hiding this comment

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

looks like this stores a password to Crowd in plain text.

/**
* The Crowd application password
*/
private String crowdApplicationPassword;
Copy link
Member

Choose a reason for hiding this comment

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

@fbelzunc did you mean to use Secret here?

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.

3 participants