passwordless-server
passwordless-server copied to clipboard
Use `ApplicationOverrides` to bypass magic link restrictions for Admin Console
We already have this mechanism so might as well use it for AdminConsole as well. This makes the IsAdminConsole
check redundant, and we don't have to hard-code the name of the app.
- Local setup: part of
appsetings.Development.json
, overrides thetest
app - Devtest, QA, Prod setup: set the
ApplicationOverrides__admin__IsRateLimitBypassEnabled=true
andApplicationOverrides__admin__IsMagicLinkQuotaBypassEnabled=true
environment variables (replaceadmin
with the actual name of the AdminConsole app)
This simplifies the initial local development setup quite a bit.
Related to https://bitwarden.atlassian.net/browse/PAS-429, although takes a different approach than described in the ticket.
Codecov Report
Attention: Patch coverage is 55.88235%
with 15 lines
in your changes are missing coverage. Please review.
Project coverage is 32.51%. Comparing base (
a2ea77a
) to head (9f95738
).
Additional details and impacted files
@@ Coverage Diff @@
## main #565 +/- ##
==========================================
- Coverage 32.56% 32.51% -0.05%
==========================================
Files 525 524 -1
Lines 26533 26529 -4
Branches 858 859 +1
==========================================
- Hits 8640 8627 -13
- Misses 17774 17781 +7
- Partials 119 121 +2
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@jrmccannon something I noticed later: it seems that, currently, AC will redirect to /initialize
, even if the data is already seeded. I assume it was changed recently after our call. Since the seeded data is now valid and we don't have to go through the /initialize
sequence in local development, can we re-add a check that lets us avoid the initialization sequence? If so, could you advise how to do it?
@jrmccannon something I noticed later: it seems that, currently, AC will redirect to
/initialize
, even if the data is already seeded. I assume it was changed recently after our call. Since the seeded data is now valid and we don't have to go through the/initialize
sequence in local development, can we re-add a check that lets us avoid the initialization sequence? If so, could you advise how to do it?
What should currently happen is that AC will check to see if its own database has had a migration ran. If there have been any migrations applied (database initialized), then it redirects to Login or Overview. Otherwise we go to Initialize.
When you say the data has been seeded, are you talking about admin console existing in API or within its own database?
The initialization step in AC sets up the AdminConsole application in API, runs all migrations (creating the database) for AC, then adds AC as an application and organization to itself.
Having all this done ensures both AC and API are in valid states and running like every other application.
Which part would we need to skip and where?
@jrmccannon something I noticed later: it seems that, currently, AC will redirect to
/initialize
, even if the data is already seeded. I assume it was changed recently after our call. Since the seeded data is now valid and we don't have to go through the/initialize
sequence in local development, can we re-add a check that lets us avoid the initialization sequence? If so, could you advise how to do it?What should currently happen is that AC will check to see if its own database has had a migration ran. If there have been any migrations applied (database initialized), then it redirects to Login or Overview. Otherwise we go to Initialize.
When you say the data has been seeded, are you talking about admin console existing in API or within its own database?
The initialization step in AC sets up the AdminConsole application in API, runs all migrations (creating the database) for AC, then adds AC as an application and organization to itself.
Having all this done ensures both AC and API are in valid states and running like every other application.
Which part would we need to skip and where?
Let me explain. With the changes in this PR, the startup flow looks like this:
- Run API
- Apply migrations & seed data in the API
- Run AdminConsole
- Going to the home page immediately redirects to the
/initialize
page, where it asks me to create a new Admin account
With the Admin app being seeded by the API, I believe we'd only need to migrate the DB in AC and it should be good to go, without the need to create a new Admin account.
Maybe AC can check if the provided keys in the configuration are valid (by sending a test request) and only redirect to the /initialize
page if they are not, and just perform migrations if they are.
What do you think @jrmccannon?
Ok done
Did we validate that self-hosting starts up and runs fine with these changes?
I couldn't get it to run locally using run_self-hosting-image.sh
:
That said, isn't self-host running in production environment, not development? In which case it shouldn't be affected.