Add option to force authentication#2861
Add option to force authentication#2861chrisknoll merged 5 commits intoOHDSI:masterfrom RowanErasmus:master
Conversation
chrisknoll
left a comment
There was a problem hiding this comment.
Looks good, but make 2 changes to behavior:
- Rename flag to
enableSkipLoginwhich will automatically attempt to open the login when your authentication fails inloadUserInfo(). - Remove the check to the flag in
welcome.jsso that if you only have 1 provider, you invoke it automatically (we don't need a flag for this behavior).
|
@chrisknoll as agreed upon during yesterdays meeting I have renamed the flag, made the logon with 1 provider always automatically go by default and as agreed I hit autoformat on the all over the place alligned app.js I very much second this OHDSI/WebAPI#2203 btw, automagically consistently formatted code is a blessing well worth that initial single mega pull request in my opinion ;-) |
|
Thank,s @RowanErasmus. I apologize, i was looking at the other flags, and other start with enableXXX, so can we change skipLoginEnable to enableSkipLogin? Sorry for the back and forth. |
|
@chrisknoll I also set the default to false, that seemed to have slipped through... cacheSources |
|
Thank you for your patience. This probably feels pretty arbitrary since the rules are all over the place, and I didn't notice the ones that were xxEnabled but I think starting with enable makes more sense in the long run and we can think about a PR to normalize that one later. I also appreciate changing default to false because we want to maintain backwards behavior where it makes sense and have people 'opt in' to new behavior (if they are given the choice). This looks good, thanks very much. |
|
Can you do me a favor: can you go through the issues that are opened that can be closed with this change? I had some trouble trying to determine if the referenced issues should be closed, or if this was a stand-alone feature (bypass clicking on the authentication provider) and the other issues are still open. |
|
Closed one of the issues, the other has not really been resolved as the person raising it was asking for something completely different ;-) |
This is half related to these two issues:
##2857
#OHDSI/WebAPI#2268
The idea is to automatically log in to Atlas when a user is already authenticated with SSO.
To this end I have added a configuration userAuthenticationForced (better suggestions are welcome)
If this is enabled and a user is not logged in it will open the sign in modal, if there is only one auth provider it will trigger that one. As a result if a session is already established with your provider it will just log you in without any clicks.
If you have multiple outlook accounts and are using Azure AD it plays nicely with #OHDSI/WebAPI#2254 to suggest which account it should be using
A video of what it looks like when not yet authenticated:
https://github.com/OHDSI/Atlas/assets/74538318/b1c93559-39a5-4602-b8a8-6858a0ef2ec9
If you have already have a session with your provider but not with Atlas it goes like so:
https://github.com/OHDSI/Atlas/assets/74538318/4595193d-a7f1-4809-adb4-e9d93a6479fd
It is not very 'smooth' and takes a couple of seconds but it's good enough for me :-)
(I've also fixed some indentation in the welcome.js file, although it still looks a bit weird in the pr...)