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

Bug in OAuth2 First Login page not using prefilled username from OAuth2UserRecord #6690

Closed
poikilotherm opened this issue Feb 26, 2020 · 6 comments

Comments

@poikilotherm
Copy link
Contributor

poikilotherm commented Feb 26, 2020

While playing with our new Jülich DATA Beta service, I discovered that the username set in the authentication provider is never used in the first login form. This should be changed.

image
image

Digging into the code. I set the field during auth:

OAuth2UserRecord getUserRecord(UserInfo userInfo) {
return new OAuth2UserRecord(
this.getId(),
userInfo.getSubject().getValue(),
userInfo.getPreferredUsername(),
null,

But the view is simply requesting the username attribute, which is never set in OAuth2FirstLoginPage.init():

<div class="col-sm-4">
<p:inputText id="username" styleClass="form-control" value="#{OAuth2FirstLoginPage.username}" validator="#{OAuth2FirstLoginPage.validateUserName}"/>
<p:message for="username" display="text"/>
</div>

As always most of the time: happy to contribute. This looks like it was not left out on purpose, so dunno if we need a trigger for UI team or just go ahead and fix it.

@pdurbin
Copy link
Member

pdurbin commented Feb 26, 2020

@poikilotherm as always, thanks for the bug report! Please go ahead and try to fix it! Thanks! 😄

@espenfl
Copy link

espenfl commented Nov 12, 2020

Dear everyone. We also noticed this upon enabling OIDC for our organization. Would be nice to fill the user name as well. By the way, would @ in the user name be supported?

@pdurbin
Copy link
Member

pdurbin commented Nov 12, 2020

By the way, would @ in the user name be supported?

@espenfl you are welcome to open an issue about this. Here's the code we use: https://github.com/IQSS/dataverse/blob/v5.2/src/main/java/edu/harvard/iq/dataverse/UserNameValidator.java#L36

@pdurbin
Copy link
Member

pdurbin commented Jan 6, 2023

This issue should be easier to work on now that developers have a simple way to spin up Keycloak on their laptops:

By the way, @espenfl since I'm leaving a comment anyway, I'm not sure if you ever created a dedicated issue about supporting @ in usernames, but if you're still interested in this, please go ahead and create one (and please comment here with the issue number). Thanks.

@RemieJanssen
Copy link

We use a OIDC connection for auth in our Dataverse, and we are also interested in having the username prefilled.
(In fact, we would prefer to skip this 'first login' form altogether, and rely on the information from the OIDC connection)

I dug through the code some more and found this comment

public void setNewUser(OAuth2UserRecord newUser) {
this.newUser = newUser;
// uncomment to suggest username to user
//setUsername(newUser.getUsername());
setSelectedEmail(newUser.getDisplayInfo().getEmailAddress());
}

I guess that the setNewUser method is still called from the following line, but I'm not so familiar with the code base yet...

This would suggest that uncommenting the line setUsername(newUser.getUsername()); might solve the problem. Looking at the blame, it seems @pdurbin has disabled this about 8 years ago. Do you recall why, and do you still stand by that?

@cmbz
Copy link

cmbz commented Aug 20, 2024

To focus on the most important features and bugs, we are closing issues created before 2020 (version 5.0) that are not new feature requests with the label 'Type: Feature'.

If you created this issue and you feel the team should revisit this decision, please reopen the issue and leave a comment.

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

No branches or pull requests

5 participants