feat: Add loginUsername support to CSV user import for external OAuth #49
Conversation
Coverage Report
|
| CANCEL_BUTTON_TEXT: "Cancel", | ||
| IMPORT_USERS_HELP_TEXT: <div>Upload a CSV file with the following header:<br/><br/> | ||
| "userId","displayName","role","groups"<br/> | ||
| "userId","displayName","role","groups","loginUsername"<br/> |
There was a problem hiding this comment.
Should we add loginUsername after userId since it is semantically closer to userId?
| } | ||
|
|
||
| userRepository.save(getNewUserEntity(userId, displayName, role, false, null, false)); | ||
| userRepository.save(getNewUserEntity(userId, null, displayName, role, false, null, false)); |
There was a problem hiding this comment.
Pass userId as default loginUsername
ustraria
force-pushed
the
wip/ustraria/import-users-login-username
branch
from
21c5e6e to
a7a1adf
Compare
Coverage Report
|
There was a problem hiding this comment.
loginUsername still at the end? Didn't we have to update the tests?
There was a problem hiding this comment.
Tests were updated because of change where loginUsername defaults to userId instead of null. For the CSV parsing the column order doesn't matter due to the @CsvBindByName annotations, which map columns by header name , not by position.
| continue; | ||
| } | ||
|
|
||
| String loginUsername = userId; |
There was a problem hiding this comment.
Wondering if we should add the validations similar to what you added for default groupIds for the importUsers values since they will also not be coming from the web client
There was a problem hiding this comment.
The validations are not consistent between the values from importUsers and the claims. I don't think there is validation on loginUsername besides character length from claims or in importUsers. For groups in importUsers there is also not validation. We could add validation for groups for importUsers to match?
2 participants
Issue #, if available:
Description of changes:
Changes:
Why is this change necessary:
For external OAuth providers like AWS Cognito, userId is typically a UUID (sub claim) while loginUsername is the human-readable username. This change enables pre-provisioning users with their loginUsername via CSV import, which is useful for setting up user permissions before their first login.
How was this change tested:
Manually tested:
Any additional information or context required to review the change:
Documentation Checklist:
import-users-constants.tsxwhich has instructions.Compatibility Checklist:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.