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

unexpected behavior when disabling built-in user authentication #4267

Closed
pameyer opened this issue Nov 8, 2017 · 11 comments · Fixed by #6105
Closed

unexpected behavior when disabling built-in user authentication #4267

pameyer opened this issue Nov 8, 2017 · 11 comments · Fixed by #6105

Comments

@pameyer
Copy link
Contributor

pameyer commented Nov 8, 2017

For an installation with OAuth only authentication, disabling built-in users results in unexpected behavior in the login page:

before disabling built-in accounts

screen shot 2017-11-08 at 1 27 22 pm

[root@dv-dev ~]# curl -X GET http://localhost:8080/api/admin/authenticationProviders | jq .
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
120 481 120 481 0 0 9268 0 --:--:-- --:--:-- --:--:-- 10688
{
"data": [
{
"enabled": true,
"factoryData": "type: XXXXX | userEndpoint: XXXXXX | clientId: XXXXXX | clientSecret: XXXXX",
"subtitle": "",
"title": "ORCID",
"factoryAlias": "oauth2",
"id": "orcid-sandbox"
},
{
"enabled": true,
"factoryData": "",
"subtitle": "Datavers' Internal Authentication provider",
"title": "Dataverse Local",
"factoryAlias": "BuiltinAuthenticationProvider",
"id": "builtin"
}
],
"status": "OK"
}

disable built-in account:

[root@dv-dev ~]# curl -X PUT -d 'false' http://localhost:8080/api/admin/authenticationProviders/buil
tin/enabled

after disabling:

screen shot 2017-11-08 at 1 27 50 pm

{"status":"OK","data":{"mGET http://localhost:8080/api/admin/authenticationProviders | jq .
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
120 482 120 482 0 0 28656 0 --:--:-- --:--:-- --:--:-- 48200
{
"data": [
{
"enabled": true,
"factoryData": "type: orcid | userEndpoint: XXXX | clientId: XXXX | clientSecret: XXXX",
"subtitle": "",
"title": "ORCID",
"factoryAlias": "oauth2",
"id": "orcid-sandbox"
},
{
"enabled": false,
"factoryData": "",
"subtitle": "Datavers' Internal Authentication provider",
"title": "Dataverse Local",
"factoryAlias": "BuiltinAuthenticationProvider",
"id": "builtin"
}
],
"status": "OK"
}

@pdurbin
Copy link
Member

pdurbin commented Nov 8, 2017

@pameyer does restarting Glassfish help?

@pdurbin
Copy link
Member

pdurbin commented Nov 8, 2017

Oh, "false" doesn't do anything. You have to delete the built in provider. I think I documented this but you can fact check me. 😄

@pdurbin
Copy link
Member

pdurbin commented Nov 8, 2017

I'm looking at what I wrote and it's confusing. As of http://guides.dataverse.org/en/4.8.2/installation/config.html#auth-modes-local-vs-remote-vs-both it says this:


"Remote only" mode should be considered experimental until #2974 is resolved. For now, "remote only" means:

  • Shibboleth or OAuth has been enabled.
  • :AllowSignUp is set to "false" per the :doc:config section to prevent users from creating local accounts via the web interface. Please note that local accounts can also be created via API, and the way to prevent this is to block the builtin-users endpoint or scramble (or remove) the BuiltinUsers.KEY database setting per the :doc:config section.
  • The "builtin" authentication provider has been disabled. Note that disabling the builting auth provider means that the API endpoint for converting an account from a remote auth provider will not work. This is the main reason why Login page for "only remote authentication" mode #2974 is still open. Converting directly from one remote authentication provider to another (i.e. from GitHub to Google) is not supported. Conversion from remote is always to builtin. Then the user initiates a conversion from builtin to remote. Note that longer term, the plan is to permit multiple login options to the same Dataverse account per Permit multiple login options to the same Dataverse account #3487 (so all this talk of conversion will be moot) but for now users can only use a single login option, as explained in the :doc:/user/account section of the User Guide. In short, "remote only" might work for you if you only plan to use a single remote authentication provider such that no conversion between remote authentication providers will be necessary.

Maybe I'm wrong about that false/enabled thing. I don't know. Again, I think you have to delete the whole auth provider and restart Glassfish. If that doesn't work I would suggest reading through #2974.

@pdurbin
Copy link
Member

pdurbin commented Nov 8, 2017

I see that over at #2974 (comment) I wrote, "The main code change I added is to not show the suggestion to convert your account if you have deleted the builtin auth provider."

@pameyer
Copy link
Contributor Author

pameyer commented Nov 8, 2017

restarting glassfish didn't help; and deleting the build-in provider didn't help. :AllowSignUp was at no , changing to false didn't change behavior.

Guessing this is related to the id="otherProviders" jsf:rendered="#{LoginPage.listAuthenticationProviders().size() > 1}">; will investigate further

@pameyer
Copy link
Contributor Author

pameyer commented Nov 8, 2017

Initial guess was incorrect. isOAuthProvider should be called from jsf:rendered="#{LoginPage.authP ; appears to be called for built-in users (returning false, as it should); appears not to be called (at least, does not trigger breakpoint) for OAuth providers (nope, wasn't that either).

@pdurbin
Copy link
Member

pdurbin commented Nov 21, 2017

Today I let @pameyer know about :DefaultAuthProvider at http://guides.dataverse.org/en/4.8.3/installation/config.html#auth-modes-local-vs-remote-vs-both which might mitigate the problem. Or might not. We'll see. 😄

@pameyer
Copy link
Contributor Author

pameyer commented Nov 21, 2017

@pdurbin Thanks for the suggestion - this does appear to work.
screen shot 2017-11-21 at 5 16 10 pm

@djbrooke
Copy link
Contributor

Thanks @pdurbin !

@pdurbin
Copy link
Member

pdurbin commented Nov 21, 2017

Sure. As I said at #4267 (comment) the documentation is confusing, so maybe this issue should be about cleaning it up.

@pdurbin
Copy link
Member

pdurbin commented Aug 21, 2019

@pameyer heads up that I just marked pull request #6105 as closing this issue. I guess I'll go request a review from you. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants