Fix username collision #6
Conversation
Signed-off-by: Bastien Wermeille <[email protected]>
Built from the either the service name if present or the login service.
The first step is to ensure that the user has the proper prefix otherwise the removeprefix function will just return the same value and thus the check can continue. Hence if the wrong prefix is present, do not allow the user at all.
The test extra only installs test related dependencies and thus pre-commit is missing.
|
There's an extreme corner case where if the login service name has a colon in this would behave incorrectly. Perhaps the easiest solution is to require that |
|
@manics do you mean not use any separator between the prefix and the original username ? |
|
Sorry, I meant ensure that |
|
@manics no worries, it's all good now. |
|
@manics thanks for the suggestions ! I just realized something, I may have over-engineered That would be something to be done in a different MR though. |
|
I can think of arguments both ways. Using an existing property means less configuration, whereas using a new property means admins can customise the resulting username (e.g. they may prefer a short prefix instead of the longer name), and makes it easier to add Authenticators that aren't related to OAuthenticator. |
login_service is to a configurable traits hence service_name is more adequate. The alternative is to create a subclass and change login_service there.
This is wrong and rightfully triggers a warning that the user shall fix in their code. There's no reason to check the behaviour for it in this class.
The two cases have been separated and tested for the various way an invalid name can be created and set.
|
@manics good points, I have refactored and improved the tests with that in mind. |
This is a PR to document the solution proposed here jupyterhub/oauthenticator#459 and to discuss it.