feat: run local development against a mock identity provider - #97
Merged
Conversation
The dev environment disabled the main firewall outright, because AAK's OIDC setup has no redirect URI for itksites.local.itkdev.dk. That meant no OpenID Connect code path was exercised until it reached a server — a broken callback path, a renamed claim or a login loop all stayed invisible locally — and the setting is easy to forget, so an authentication problem could be debugged for a long time against a firewall that was never running. A mock provider gives the whole authorization code flow locally instead. The container name is the external hostname on purpose, so the browser and the application reach it by the same name and the issuer matches. .env.dev carries the settings, so an ordinary setup needs nothing in .env.local. AZURE_AZ_OIDC_ALLOW_HTTP is true there and must not be anywhere else: from itk-dev/openid-connect 5.1 it governs every endpoint the discovery document announces, not only the metadata URL.
4 tasks
API Specification - Non-breaking changesNo changelog changes |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #97 +/- ##
=============================================
+ Coverage 37.14% 37.55% +0.40%
- Complexity 948 1104 +156
=============================================
Files 133 146 +13
Lines 2972 3499 +527
=============================================
+ Hits 1104 1314 +210
- Misses 1868 2185 +317
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to ticket
TODO: no ticket linked yet — please add before review.
Description
Local development ran with the main firewall switched off:
The reason was real — AAK's provider has no redirect URI registered for a developer machine — but the shape of the workaround is the problem. No OpenID Connect code path is exercised until the code reaches a server, so a broken callback path, a renamed claim or a login loop all stay invisible locally. And because it is a single line in a
when@devblock, it is easy to forget: an authentication problem can be debugged for a long while against a firewall that was never running. That is roughly what happened to me while validating #92 — several wrong turns before noticing security was off.This replaces it with a mock provider, so the whole authorization code flow runs locally.
idpservice indocker-compose.override.yml, so it never starts on a server.oidc-provider-mock, the same one deltag.aarhus.dk uses.when@devfirewall bypass removed..env.devcarries the mock settings, so an ordinary setup needs nothing in.env.local.Two identities,
adminandeditor. Their claims includenameandupnbecause that is whatAzureOIDCAuthenticatorreads — a mock without those fails at the array access rather than at anything informative.The container name is the external hostname on purpose (
idp.${COMPOSE_DOMAIN}). The browser and the application then reach the provider by the same name, so the issuer in the discovery document matches the one in the ID token. Getting this wrong gives an issuer mismatch that reads like a bundle bug.Verified
Run against this branch:
phpfpmby the external hostnameidp.itksites.local.itkdev.dk/adminanonymous302 → /openidconnect/login/azure_az— the firewall challenges again302 →the mock,scope=openid email profile, correctredirect_uri, state storedallow_httpChecklist
No new test cases: this changes the development environment only. The existing suite is unaffected —
testalready had security enabled and its own.env.test.Additional comments or questions
Existing developers must prune
.env.local. Dotenv loads.env→.env.dev→.env.local, so real Azure values already in.env.localstill win and the mock will not be used. Remove theAZURE_AZ_OIDC_*lines from it, or keep them deliberately to develop against the real provider.AZURE_AZ_OIDC_ALLOW_HTTP=trueis in.env.devand must not spread. Fromitk-dev/openid-connect5.1 it governs every endpoint the discovery document announces, not just the metadata URL, so it is a wider switch than it used to be..envkeepsfalse, and the README says so.The mock does not verify PKCE. It accepts the challenge and the verifier, so a login completes, but it omits
code_challenge_methods_supportedfrom its discovery document. Once the bundle sends PKCE by default (6.1), a green local login will not be evidence that PKCE works against Azure. The README says this.Relationship to #92. Independent — that one is the 6.0 upgrade, this is the development environment — so either can merge first. Raised as its own PR rather than added to #92, which is large enough.