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

Version-View: Editor names are not human readable #291

Open
ValentinFutterer opened this issue Nov 25, 2024 · 3 comments · May be fixed by #322
Open

Version-View: Editor names are not human readable #291

ValentinFutterer opened this issue Nov 25, 2024 · 3 comments · May be fixed by #322
Assignees
Labels
bug Something isn't working NPR New Partner Request usability improvement usability improvement

Comments

@ValentinFutterer
Copy link
Collaborator

Describe the bug
The names of editors are taken from the SecurityService, which in turn takes them from the OidcJwtCallerPrincipal which gets the name from the Jwt token. This library tries different fields in the token. First the upn (User Principal Name), which is structured similar to an email. If it cannot get the upn it tries the preferred_username, which is most likely a human-readable username in a system. Lastly, it tries to use the sub (Subject), which is an identifier number. Depending on the identity provider, different values are returned here, and most of them are not very human readable.

Is this a regression?
No.

Expected behavior
The editor should be at least the human name of the editor (firstname lastname). The maybe we can add some other differential features to avoid naming conflicts. Maybe ask Zeno how he would handle this, since he works with the tiss API, he could have some good ideas. Additionally, think about a solution of how to apply this change retroactively to the editors of old versions. Otherwise, there will be an abrupt break in naming scheme. Also think about name changes in the CRIS systems and how to handle those.

Screenshots
This is from the TU Wien DMP Tool. in DAMAP it would just say user.
image

Environment

  • Damap version 4.3.0
@ValentinFutterer ValentinFutterer added bug Something isn't working usability improvement usability improvement NPR New Partner Request labels Nov 25, 2024
@GeoffreyKarnbach GeoffreyKarnbach self-assigned this Nov 26, 2024
@ZenoLC
Copy link
Contributor

ZenoLC commented Jan 7, 2025

There is an additional issue with the Editor field:
The Hashes or Names stored in this property do not correspond with the person doing the editing.

The application will store a different user when a version is being saved.
This can be confirmed quickly by changing user and saving a version.

@ZenoLC
Copy link
Contributor

ZenoLC commented Jan 7, 2025

After checking the issue on a deeper level i was able to ascertain that the revision table (REVINFO) is maintained correctly.
The hash is still there but the CHANGED_BY_ID stored does correspond with the person making the change.

The DMP_VERSION table instead will link to a different revision object. Also several links to revisions in the VERSION table are the same, which should not be possible.

@ZenoLC
Copy link
Contributor

ZenoLC commented Jan 7, 2025

Additionally in order to fully clear this issue the existing tables need to be corrected or some of its content deleted.
What will depend on the exact issue at hand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working NPR New Partner Request usability improvement usability improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants