Skip to content
This repository has been archived by the owner on Jan 24, 2023. It is now read-only.

SSO: Improvements/refinements #2522

Merged
merged 21 commits into from
Sep 4, 2018
Merged

SSO: Improvements/refinements #2522

merged 21 commits into from
Sep 4, 2018

Conversation

nwmac
Copy link
Contributor

@nwmac nwmac commented Jun 26, 2018

This PR improves/refines the SSO support:

  • Backend logs SSO config on startup
  • Added SSO_OPTIONS config env var and support for:
    • "nosplash" value that skips the Stratos login screen and redirects to the UAA without the user having to click the login button
    • "logout" value - if specified, logging out of Stratos also logs the user out of the UAA and redirects back to Stratos when logout has completed
  • Fixes the error handling, so that instead of getting a JSON document shown in the browser, the user is redirected back to Stratos with an error message
  • Updates the backend to only add the SSO routes if SSO login is enabled

@nwmac nwmac added the V2 label Jun 26, 2018
@nwmac nwmac self-assigned this Jun 26, 2018
@cfdreddbot
Copy link

Hey nwmac!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@codecov
Copy link

codecov bot commented Jun 26, 2018

Codecov Report

Merging #2522 into v2-master will increase coverage by <.01%.
The diff coverage is 65.51%.

@@              Coverage Diff              @@
##           v2-master    #2522      +/-   ##
=============================================
+ Coverage      71.34%   71.34%   +<.01%     
=============================================
  Files            604      604              
  Lines          25868    25894      +26     
  Branches        5861     5865       +4     
=============================================
+ Hits           18455    18475      +20     
- Misses          7413     7419       +6

@nwmac nwmac changed the title SSO Improvements/refinemets SSO Improvements/refinements Jun 26, 2018
@andrewghobrial
Copy link
Contributor

@nwmac

  1. get rid of "!" in if !p.Config.SSOLogin { when checking whether to add SSO routes
  2. get rid of the extra _, err := c.portalProxy.DoLoginToCNSI(context, cfCnsi.GUID, false) on line 142 in plugins/cloudfoundry/main.go. It's doing a double login and leaving an error message in the console. I think this was accidentally added in the merge with my PR.
  3. I am not too sure what's going on with the SSO logout. There's definitely a bug in the way you are forming the url. Here's how it's showing up for me:

https://login.andrew-cfee-cluster.us-south.containers.mybluemix.net:2794/UAALoginServerWAR/logout.jsp?redirect=https://localhost:4200/pp/v1/auth/sso_login_callback?state=logout

It definitely shouldn't be including the sso_login_callback in the redirect.
4. SSO logout should be an sso option because I don't necessarily want to logout the user globally when logging out of stratos.

@nwmac nwmac added needs attention This PR needs attention and removed ready for review labels Jun 27, 2018
@andrewghobrial
Copy link
Contributor

@nwmac any progress on this? I'm interested in all these things except SSO logout. I think that can cause problems with the session of other web applications that use the same SSO flow. I think on logout, we should just redirect back to stratos login page, taking care to remove the "nosplash" option if it exists. That way it won't re-sign in.

@nwmac
Copy link
Contributor Author

nwmac commented Jul 31, 2018

@aeijdenberg No progress, but now v2 testing is done, I will give this PR some TLC and address your comments so it can be merged.

@nwmac nwmac removed the V2 label Aug 14, 2018
# Conflicts:
#	src/frontend/app/features/login/login-page/login-page.component.ts
#	src/frontend/app/store/reducers/auth.reducer.ts
#	src/jetstream/auth.go
#	src/jetstream/repository/interfaces/structs.go
@nwmac nwmac added in progress and removed needs attention This PR needs attention labels Aug 23, 2018
@nwmac nwmac changed the title SSO Improvements/refinements SSO: Improvements/refinements Aug 31, 2018
@irfanhabib
Copy link
Contributor

LGTM

@irfanhabib irfanhabib merged commit 0612282 into v2-master Sep 4, 2018
@irfanhabib irfanhabib deleted the sso-improvements branch September 4, 2018 08:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants