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

Update user data on login #2096

Closed
dasGoogle opened this issue Dec 20, 2023 · 5 comments
Closed

Update user data on login #2096

dasGoogle opened this issue Dec 20, 2023 · 5 comments
Labels
[C] Backend Focuses on backend implementation [P] Minor Minor priority [S] Very small This issue should require only very small changes.

Comments

@dasGoogle
Copy link

Just another idea that comes from the FSR KeyCloak migration process:

The new FSR Keycloak offers first and last names that we try to keep updated with the actual names of the students. Thus, it might be feasible to update the display names on every login instead of only when creating the user. This would allow students to perform one name change via the KeyCloak for all FSR-related services.

EvaP could still keep its self-service display name override feature so that names that were already changed remain correct.

CC @lukasrad02 @jeriox

@richardebeling
Copy link
Member

Sounds good to me. We could do logged updates of first_name_given (and maybe last_name) here:

def update_user(self, user, claims):
if not user.first_name_given:
user.first_name_given = claims.get("given_name", "")
user.save()
if not user.last_name:
user.last_name = claims.get("family_name", "")
user.save()
return user

If we don't touch first_name_chosen, the name a user set on EvaP would always overwrite any updated values for first_name_given, which I think would be reasonable.

@janno42 janno42 changed the title Discussion: Use display names from new KeyCloak Update user data on login Dec 20, 2023
@janno42
Copy link
Member

janno42 commented Dec 20, 2023

On login, first_name_given and last_name should be updated if the values for a user (identified by the same email) have changed.

A change in email can't be handled by EvaP and a user profile merge must be done manually in this case.

@janno42 janno42 added [C] Backend Focuses on backend implementation [P] Minor Minor priority [S] Very small This issue should require only very small changes. labels Dec 20, 2023
@janno42
Copy link
Member

janno42 commented Feb 27, 2024

closing. name changes will be handled via the campus management system import instead.

@janno42 janno42 closed this as completed Feb 27, 2024
@jeriox
Copy link

jeriox commented Feb 28, 2024

closing. name changes will be handled via the campus management system import instead.

maybe we should verify if our requirement to have a self-service for the first name in the CMS is actually going to be implemented (cc @SilvanVerhoeven)

@lukasrad02
Copy link

maybe we should verify if our requirement to have a self-service for the first name in the CMS is actually going to be implemented

I don't think that this is necessary (nevertheless, it would be nice to have, independent from EvaP), as EvaP already offers a self-service to change the first name. Imports from the CMS should only update first_name_given, but not first_name_chosen, so users that do not have their name set in EvaP will always be shown with the most recent name from the CMS, and for users that have their name set in EvaP, the CMS import does not matter at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C] Backend Focuses on backend implementation [P] Minor Minor priority [S] Very small This issue should require only very small changes.
Development

No branches or pull requests

5 participants