SSO - Ability to specify default third_party_auth provider via query param #8591
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
|
|
||
| import logging | ||
| import json | ||
| import urlparse | ||
|
|
||
| from django.conf import settings | ||
| from django.contrib import messages | ||
|
|
@@ -77,12 +78,26 @@ def login_and_registration_form(request, initial_mode="login"): | |
| if ext_auth_response is not None: | ||
| return ext_auth_response | ||
|
|
||
| # Our ?next= URL may itself contain a parameter 'tpa_hint=x' that we need to check. | ||
| # If present, we display a login page focused on third-party auth with that provider. | ||
| third_party_auth_hint = None | ||
| if '?' in redirect_to: | ||
| try: | ||
| next_args = urlparse.parse_qs(urlparse.urlparse(redirect_to).query) | ||
| provider_id = next_args['tpa_hint'][0] | ||
| if third_party_auth.provider.Registry.get(provider_id=provider_id): | ||
| third_party_auth_hint = provider_id | ||
| initial_mode = "hinted_login" | ||
|
Contributor
There was a problem hiding this comment. Question: does it make sense to have this as a query param, rather than as a separate url?
Contributor
Author
There was a problem hiding this comment. My thought was that a query param is easier for partner universities to construct - just copy the URL to some part of the courseware, then append the query string part. If it's a separate URL we'd probably need to create a UI/tool that creates the URL for them, no? And there may be many different types of links that we'd need to accommodate - so several new URL patterns may be required. Edit: I guess something like
Contributor
There was a problem hiding this comment. I that makes sense. I was thinking there was just one url, but if the idea is to just have any of our normal urls, but decorate with an optional query param, then that sounds fine. |
||
| except (KeyError, ValueError, IndexError): | ||
| pass | ||
|
|
||
| # Otherwise, render the combined login/registration page | ||
| context = { | ||
| 'login_redirect_url': redirect_to, # This gets added to the query string of the "Sign In" button in the header | ||
| 'disable_courseware_js': True, | ||
| 'initial_mode': initial_mode, | ||
| 'third_party_auth': json.dumps(_third_party_auth_context(request, redirect_to)), | ||
| 'third_party_auth_hint': third_party_auth_hint or '', | ||
| 'platform_name': settings.PLATFORM_NAME, | ||
| 'responsive': True, | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| define([ | ||
| 'jquery', | ||
| 'underscore', | ||
| 'common/js/spec_helpers/template_helpers', | ||
| 'common/js/spec_helpers/ajax_helpers', | ||
| 'js/student_account/views/HintedLoginView', | ||
| ], function($, _, TemplateHelpers, AjaxHelpers, HintedLoginView) { | ||
| 'use strict'; | ||
| describe('edx.student.account.HintedLoginView', function() { | ||
|
|
||
| var view = null, | ||
| requests = null, | ||
| PLATFORM_NAME = 'edX', | ||
| THIRD_PARTY_AUTH = { | ||
| currentProvider: null, | ||
| providers: [ | ||
| { | ||
| id: 'oa2-google-oauth2', | ||
| name: 'Google', | ||
| iconClass: 'fa-google-plus', | ||
| loginUrl: '/auth/login/google-oauth2/?auth_entry=account_login', | ||
| registerUrl: '/auth/login/google-oauth2/?auth_entry=account_register' | ||
| }, | ||
| { | ||
| id: 'oa2-facebook', | ||
| name: 'Facebook', | ||
| iconClass: 'fa-facebook', | ||
| loginUrl: '/auth/login/facebook/?auth_entry=account_login', | ||
| registerUrl: '/auth/login/facebook/?auth_entry=account_register' | ||
| } | ||
| ] | ||
| }, | ||
| HINTED_PROVIDER = "oa2-google-oauth2"; | ||
|
|
||
| var createHintedLoginView = function(test) { | ||
| // Initialize the login view | ||
| view = new HintedLoginView({ | ||
| thirdPartyAuth: THIRD_PARTY_AUTH, | ||
| hintedProvider: HINTED_PROVIDER, | ||
| platformName: PLATFORM_NAME | ||
| }); | ||
|
|
||
| // Mock the redirect call | ||
| spyOn( view, 'redirect' ).andCallFake( function() {} ); | ||
|
Contributor
Author
There was a problem hiding this comment. True; I had just copy-pasted that line from |
||
|
|
||
| view.render(); | ||
| }; | ||
|
|
||
| beforeEach(function() { | ||
| setFixtures('<div id="hinted-login-form"></div>'); | ||
| TemplateHelpers.installTemplate('templates/student_account/hinted_login'); | ||
| }); | ||
|
|
||
| it('displays a choice as two buttons', function() { | ||
| createHintedLoginView(this); | ||
|
|
||
| expect($('.proceed-button.button-oa2-google-oauth2')).toBeVisible(); | ||
| expect($('.form-toggle')).toBeVisible(); | ||
| expect($('.proceed-button.button-oa2-facebook')).not.toBeVisible(); | ||
| }); | ||
|
|
||
| it('redirects the user to the hinted provider if the user clicks the proceed button', function() { | ||
| createHintedLoginView(this); | ||
|
|
||
| // Click the "Yes, proceed" button | ||
| $('.proceed-button').click(); | ||
|
|
||
| expect(view.redirect).toHaveBeenCalledWith( '/auth/login/google-oauth2/?auth_entry=account_login' ); | ||
| }); | ||
| }); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| var edx = edx || {}; | ||
|
|
||
| (function($, _, gettext) { | ||
| 'use strict'; | ||
|
|
||
| edx.student = edx.student || {}; | ||
| edx.student.account = edx.student.account || {}; | ||
|
|
||
| edx.student.account.HintedLoginView = Backbone.View.extend({ | ||
| el: '#hinted-login-form', | ||
|
|
||
| tpl: '#hinted_login-tpl', | ||
|
|
||
| events: { | ||
| 'click .proceed-button': 'proceedWithHintedAuth' | ||
| }, | ||
|
|
||
| formType: 'hinted-login', | ||
|
|
||
| initialize: function( data ) { | ||
| this.tpl = $(this.tpl).html(); | ||
| this.providers = data.thirdPartyAuth.providers || []; | ||
| this.hintedProvider = _.findWhere(this.providers, {id: data.hintedProvider}) | ||
| this.platformName = data.platformName; | ||
|
|
||
| }, | ||
|
|
||
| render: function() { | ||
| $(this.el).html( _.template( this.tpl, { | ||
| // We pass the context object to the template so that | ||
| // we can perform variable interpolation using sprintf | ||
| providers: this.providers, | ||
| platformName: this.platformName, | ||
| hintedProvider: this.hintedProvider | ||
| })); | ||
|
|
||
| return this; | ||
| }, | ||
|
|
||
| proceedWithHintedAuth: function( event ) { | ||
| this.redirect(this.hintedProvider.loginUrl); | ||
| }, | ||
|
|
||
| /** | ||
| * Redirect to a URL. Mainly useful for mocking out in tests. | ||
| * @param {string} url The URL to redirect to. | ||
| */ | ||
| redirect: function( url ) { | ||
| window.location.href = url; | ||
| } | ||
| }); | ||
| })(jQuery, _, gettext); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| <div class="wrapper-other-login"> | ||
| <div class="section-title lines"> | ||
| <h2> | ||
| <span class="text"><%- gettext("Sign in") %></span> | ||
| </h2> | ||
| </div> | ||
|
|
||
| <p class="instructions"><%- _.sprintf( gettext("Would you like to sign in using your %(providerName)s credentials?"), { providerName: hintedProvider.name } ) %></p> | ||
|
|
||
| <button class="action action-primary action-update proceed-button button-<%- hintedProvider.id %> hinted-login-<%- hintedProvider.id %>"> | ||
| <div class="icon fa <%- hintedProvider.iconClass %>" aria-hidden="true"></div> | ||
| <%- _.sprintf( gettext("Sign in using %(providerName)s"), { providerName: hintedProvider.name } ) %> | ||
| </button> | ||
|
|
||
| <div class="section-title lines"> | ||
| <h2> | ||
| <span class="text"><%- gettext("or") %></span> | ||
| </h2> | ||
| </div> | ||
|
|
||
| <div class="toggle-form"> | ||
| <button class="nav-btn form-toggle" data-type="login"><%- gettext("Show me other ways to sign in or register") %></button> | ||
|
Contributor
There was a problem hiding this comment. Is there binding that I'm missing that makes this button work?
Contributor
Author
There was a problem hiding this comment. @cpennington Yep, it's an existing handler for the page as a whole in |
||
| </div> | ||
| </div> | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a note (totally don't have to fix this) but in python since you can define a string with single or double quotes, if you define it with single quotes you don't have to escape the double quotes (and vice versa).