feat: Restrict certain users from production environments #2530
Conversation
| email: string, | ||
| env?: string, | ||
| ): Promise<boolean> => { | ||
| const isStagingOnlyUser = await $api.user.isStagingOnly(email); |
There was a problem hiding this comment.
Why is this it it's own function and not a field added to user.getByEmail()?
I appreciate that this does mean an additional query but the user.isStagingOnly property is only ever needed here once on login.
If we added this to the more general query, we'd be polluting our User type with a meaningless property that has no value outside of this context.
This is one of those cases where the full database model will not match the model we want in-application.
There was a problem hiding this comment.
Makes sesne to me, thanks for explanation!
DafyddLlyr
force-pushed
the
dp/staging-only-users
branch
from
acee5a5 to
2ca9f1c
Compare
|
Removed vultr server and associated DNS entries |
See theopensystemslab/planx-new#2530 for full context
| @@ -7,6 +7,8 @@ CREATE TEMPORARY TABLE sync_users ( | |||
| created_at timestamptz, | |||
| updated_at timestamptz, | |||
| is_platform_admin boolean | |||
| -- , | |||
| -- is_staging_only boolean | |||
There was a problem hiding this comment.
are these lines intended to be comment-out in sync script?
There was a problem hiding this comment.
Yep - it's a bit of chicken and egg situation - we can't sync from the new column until it exists on prod.
Once this is across there'll be a quick PR uncommenting these lines (plus any potential sync script fixes as it's currently untestable).
There was a problem hiding this comment.
(And the catch here is that pizzas will fail unless it's commented out as they can't sync)
| email: string, | ||
| env?: string, | ||
| ): Promise<boolean> => { | ||
| const isStagingOnlyUser = await $api.user.isStagingOnly(email); |
There was a problem hiding this comment.
Makes sesne to me, thanks for explanation!
What does this PR do?
Why?
As we sync the users table across environments, this means all users can access production. Generally this is what we want as "real" users live on production and this allows them to access test and staging environments - all good.
The catch is that occasionally we have a set of users who we do not wish to grant access to production - for example the user accounts associated with the recent pen test, or certain sets of users with "demo" access (until we have these roles set up). This is not a common occurrence or user type, but it feels like a sensible restriction to put in place restrictions here as a bit of a guard rail.
TODO
user.isStagingOnly()function planx-core#219 tomainplanx-corelatest