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

Username is not corrected with ocdi user's subject #3625

Closed
stevenscn opened this issue Apr 1, 2021 · 7 comments · Fixed by #3628 or #3629
Closed

Username is not corrected with ocdi user's subject #3625

stevenscn opened this issue Apr 1, 2021 · 7 comments · Fixed by #3628 or #3629
Labels
area/portal apollo-portal bug Categorizes issue or PR as related to a bug.

Comments

@stevenscn
Copy link

stevenscn commented Apr 1, 2021

we are integrating Apollo with Keycloak (openid provider), and got below issues:

  1. it seems to be not corrected that Apollo use ocdi user's subject as user id , should be some other properties such as email/username... because the subject of ocdi user's subject may be ID in database. it's better that can be configurated by user.
  2. if the apollo work on Nginx or with some context path such as apollo, when logout (integrate with openid), it redirect to /logout, should be /apollo/logout.
@nobodyiam
Copy link
Member

  1. it seems to be not corrected that Apollo use ocdi user's subject as user id , should be some other properties such as email/username... because the subject of ocdi user's subject may be ID in database. it's better that can be configurated by user.

Do you mean oidc? @vdisk-group would you please help to take a look at this?

  1. if the apollo work on Nginx or with some context path such as apollo, when logout (integrate with openid), it redirect to /logout, should be /apollo/logout.

Please refer Portal挂载到nginx/slb后如何设置相对路径?

@stevenscn
Copy link
Author

@nobodyiam ,

  1. Yes, it is oidc.
  2. actually, i have setup nginx,and which work well without oidc, just can't work with oidc.

1 similar comment
@stevenscn
Copy link
Author

@nobodyiam ,

  1. Yes, it is oidc.
  2. actually, i have setup nginx,and which work well without oidc, just can't work with oidc.

This was referenced Apr 4, 2021
@vdiskg
Copy link
Contributor

vdiskg commented Apr 4, 2021

  1. it seems to be not corrected that Apollo use ocdi user's subject as user id , should be some other properties such as email/username... because the subject of ocdi user's subject may be ID in database. it's better that can be configurated by user.

the column Users.Username is designed as a unique identification. the Implementation of UserService#findByUserId on OidcLocalUserService and SpringSecurityUserService are just like this.

  @Override
  public UserInfo findByUserId(String userId) {
    UserPO userPO = userRepository.findByUsername(userId);
    return userPO == null ? null : userPO.toUserInfo();
  }

so add a new column for human readable username maybe better.
such as #3629

  1. if the apollo work on Nginx or with some context path such as apollo, when logout (integrate with openid), it redirect to /logout, should be /apollo/logout.

fix on #3628

@stevenscn
Copy link
Author

In my opinion, the email or login name, may be identification at most time, it is better that have user decide which property is used for identification, should keep open for variability.

@vdiskg
Copy link
Contributor

vdiskg commented Apr 4, 2021

In my opinion, the email or login name, may be identification at most time, it is better that have user decide which property is used for identification, should keep open for variability.

make the email or login name be identification can leads to security vulnerabilities.
the email or login name in oidc provider might be changed. it can break the relationships of oidc user and the local user, witch cause the authority of each application ineffective.and if an ordinary user change it's email or login name to an admin user once owned, the ordinary user may get the privilege of the admin user

@stevenscn
Copy link
Author

stevenscn commented Apr 5, 2021

  1. Any properties are seen as identity should be unique and not updatable(need validate email if user still update his email at least), both in apollo and oidc system
  2. Admin should know that which properties can be seen as identity.
  3. Suppose your solution is reasonable, but you just ensure that oidc user can login apollo. if an account exists in apollo and oidc system, has same username/email, which should be same account in business and theory, but apollo would consider it is another account with your solution.

@Anilople Anilople added area/portal apollo-portal bug Categorizes issue or PR as related to a bug. labels Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/portal apollo-portal bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants