added palomar cc to config/cloudbank.cluster.yaml #395
Conversation
| - [email protected] | ||
| - [email protected] | ||
| - [email protected] | ||
| username_pattern: '^(.+@palomar\.edu|yuvipanda@gmail\.com|choldgraf@gmail\.com|georgiana\.dolocan@gmail\.com|aculich@berkeley\.edu|sean.smorris@berkeley\.edu|deployment-service-check)$' |
There was a problem hiding this comment.
@yuvipanda could you explain what username_pattern means here? This line looks correct comparing it to the previous lines, but it's a bit hard to understand what it is and I don't see it documented
There was a problem hiding this comment.
I think @GeorgianaElena added it in #208, that PR has some context. Any ideas if we can improve this, or where we can document it, @GeorgianaElena?
There was a problem hiding this comment.
Looking at it some more, I think it's a regular expression that permits everyone from @palomar.edu, but also these explicitly listed addresses otherwise.
There was a problem hiding this comment.
When an admin should not be allowed? I mean... I was expecting the admins to be excluded from the username_pattern...
There was a problem hiding this comment.
I was expecting the admins to be excluded from the username_pattern
Me too 👀
This is coming from jupyterhub. All users need to comply with username_pattern, including admins. Some code references:
- The
username_patternis checked invalidate_user validate_useris called no matter the user type
Other thing that I noticed and I think it's related, is about allowed_users. All the admins, apart from being listed in the admin_users list, need to be in this allowed list too otherwise they will be denied access.
In my mind, if the user it's an admin, then they should be allowed by default.
(These over-complicates a bit #311 too)
I'm thinking about opening a PR on jupyterhub's side about this, but I would like to know your opinion first, in case my intuition is not right and I'm trying to create an anti-pattern.
There was a problem hiding this comment.
Jumping in a bit late here.
I did look around at entries for the other hubs in the cloudbank.cluster.yaml file. The username_pattern element was in the mills definition but not in the others. I probably should have checked out whether or not to use what appeared to be the general pattern:
allowed_users: &sbcc_users
- [email protected]
- [email protected]
- [email protected]
- [email protected]
- [email protected]
- [email protected]
admin_users: *sbcc_users
as opposed to the pattern mills is using:
admin_users: &mills_admins
- [email protected]
- [email protected]
- [email protected]
- [email protected]
- [email protected]
- [email protected]
- [email protected]
username_pattern:'^(.+@mills\.edu|yuvipanda@gmail\.com|choldgraf@gmail\.com|georgiana\.dolocan@gmail\.com|aculich@berkeley\.edu|jpercy@berkeley\.edu|deployment-service-check)$'
Are we ok here? Or should this change?
|
Am just watching https://github.com/2i2c-org/pilot-hubs/runs/2560650750?check_suite_focus=true now, should have the hub up shortly |
But if you have the allowed_users list empty, admins should be able to log in, right? cc @GeorgianaElena
I think it makes sense to have a discussion there about this 😉 |
|
If allowed_users is empty, than everyone is allowed to login. But what we want is to only allow people who are admins to log in as well, so they can add other users via the db. For things like mills, we want everyone with the mills id to be able to log in, and in addition to that we want the people who are admins to login - so maybe they don't need to be listed in username_pattern? Not sure! We're mixing two sources of users - the config and the db. This gets quite confusing... |
Yep, I got that from another comment you posted in another issue, thanks for the clarification.
Since you are defining username_pattern, I think you may need to explicitly add the admins there because it does not seem admins are "allowed" nor "patterned" by default... which I find weird at first, but I am maybe missing something else.
Maybe some explicit note in the docs could help. |
@2i2c-org/tech-team Hey folks here is a first attempt at a PR for the addition of Palomar Community College.