Modular user AuthN, support for SAML #52
Conversation
Be sure to delete authnz.pyc for local development, or you'll see wacko results going forward. :-)
Use a much more modular approach to user authentication. This will help deliver support for multiple different mechanisms. (Oauth, SAML, etc.)
I don't think any of these were deliberately executable.
- Flesh out SAML authentication module with SingleSignOn and SingleLogOut support. - Add various settings needed for SAML configuration. - Add rudimentary log out support, with logout link and goodbye page.
- Don't explode if XSRF tokens aren't found. - Be more accepting of different first_name/last_name SAML attributes. - Always clear the session immediately when initiating SingleLogOut. - Remove whitespace from base64-encoded certs/keys. - Fix SAML_SP_CERT typo.
| @@ -96,8 +74,8 @@ def check_csrf_token(): | |||
| # csrf tokens. | |||
| if g.auth_type == 'kms': | |||
| return True | |||
| token = request.headers.get('X-XSRF-TOKEN') | |||
| return safe_str_cmp(token, session.get('XSRF-TOKEN')) | |||
| token = request.headers.get('X-XSRF-TOKEN', '') | |||
There was a problem hiding this comment.
I wonder if we should check to see if token is None and return False here if so.
|
|
||
| # ideally we would call check_csrf_token, but I don't think logout CSRF | ||
| # is a serious concern for this application |
There was a problem hiding this comment.
Agreed. Not worried about mass logouts here :)
|
Mostly just style and nitpicks in my comments. I think the general approach is great! |
|
@ryan-lane is this PR planned to be in 1.1? |
|
👍 On Tue, Apr 19, 2016 at 5:00 PM, Ryan Lane notifications@github.com wrote:
|
|
Awesome work @ab. Thanks again for this! |
| since any page with @authnz.require_auth will redirect to login. | ||
| """ | ||
| return flask.redirect( | ||
| authnz.user_mod.login_redirect_url(return_to='/v1/saml/debug')) |
There was a problem hiding this comment.
Yeah it's a little weird to leave that in there, since I was using it for debugging the IdP integration. But the reason this endpoint exists is for debugging SAML. You never hit this page unless you type this URL explicitly. When tracing the HTTP requests it's nice to have separate start and end pages — a bit confusing to start and end at the same page the way the @require_auth wrapped endpoints do.
@ryan-lane This is definitely a work in progress, opening a PR to get feedback on the approach here before I do too much more. I used the 1.1 branch as the base since there was a bunch of refactoring; I hope that was a good thing to do.
Summary of changes:
log_in()andauth_type(), and may implementlog_out()or other custom functionality.USER_AUTH_MODULEin settings.NullUserAuthenticator(forUSE_AUTH=false) andGoogleOauthAuthenticator, which is mostly copy pasted from the existing code.SamlAuthenticator, which implements support for SAML. This also requires some custom routes for the callback handlers, which are all namespaced under/v1/saml/./logout, which either performs a SAML SingleLogOut flow or just clears the session. Thegoodbye.htmlis just a stub that I blindly cargo culted fromindex.html.Testing: