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

Mixed authentication modes #101

Merged
merged 1 commit into from
Aug 9, 2014
Merged

Conversation

papaben
Copy link
Contributor

@papaben papaben commented Jul 13, 2013

Configure multiple authentication methods in order. This allows users to specify the preferred method of authentication, but provide a fallback if that fails.

We would rather not create service users in ldap for the purpose of interacting with collins, and this allows us to do without affecting existing users and perhaps providing flexibility for future adopters.

One note regarding the tests --
This test passes individually (i.e. test-only util.security.MixedAuthenticationProviderSpec), but fails as part of the global play test. This is because configurations from previous tests bleed over between tests. At some point the authentication_reference.conf file is being loaded and polluting the configurations for ldap, which makes the ldap provider actually attempt a lookup and fail.

Configure multiple authentication methods in order. This allows users to specify the preferred method of authentication, but provide a fallback if that fails.
/**
* Ordered list of permitted authentication types
*/
private val configuredTypes = types.split(",").toSeq.map(_.trim)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we do a toSeq here before the map?

"a,b,c,d".split(",").map(_.trim).view

@yl3w
Copy link
Contributor

yl3w commented Jun 24, 2014

lgtm @Primer42

@byxorna
Copy link
Contributor

byxorna commented Jul 31, 2014

this LGTM as well

@byxorna byxorna self-assigned this Aug 6, 2014
@yl3w
Copy link
Contributor

yl3w commented Aug 9, 2014

Verified locally, merging. thank you very much @asheepapart

yl3w pushed a commit that referenced this pull request Aug 9, 2014
@yl3w yl3w merged commit 748485f into tumblr:master Aug 9, 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.

3 participants