Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the BankID “launch v2” client-side flow into shared template fragments and wires it into both the authenticator BankID launch page and the BankID signing consentor, controlled by new Velocity settings flags.
Changes:
- Introduces shared BankID polling and app-launcher fragments, plus per-flow “polling settings” templates to configure selectors/callbacks.
- Moves browser detection/strategy templates under
fragments/bankid/browsersand updates parsers accordingly. - Adds a new feature flag for the consentor flow (
$bankidConsentorLaunchVersion2) and integrates the shared fragments when enabled.
Reviewed changes
Copilot reviewed 10 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/identity-server/templates/core/settings.vm | Documents the new consentor launch-v2 flag (commented example override). |
| src/identity-server/templates/core/settings-defaults.vm | Adds default value for the new consentor launch-v2 flag. |
| src/identity-server/templates/core/fragments/bankid/check-platform-capabilities.js.vm | Updates parse paths to use the new shared browser strategy/detections fragments. |
| src/identity-server/templates/core/fragments/bankid/browsers/browser-strategy.js.vm | Adds shared BrowserStrategy and launch helpers used by browser detection templates. |
| src/identity-server/templates/core/fragments/bankid/browsers/available-browser-detections.vm | Updates browser detection includes to new shared fragment locations. |
| src/identity-server/templates/core/fragments/bankid/browsers/*.js.vm | Adds individual browser strategy entries (iOS/Android) for detection/launch behavior. |
| src/identity-server/templates/core/fragments/bankid/bankid-poller.vm | Refactors poller into a shared fragment with injected settings and optional POST payload support. |
| src/identity-server/templates/core/fragments/bankid/bankid-app-launcher.vm | Refactors app launcher into a shared fragment driven by injected polling settings. |
| src/identity-server/templates/core/authenticator/bankid/launch/index.vm | Switches BankID launch-v2 flow to shared poller/app-launcher fragments and adds settings parse. |
| src/identity-server/templates/core/authenticator/bankid/launch/bankid-polling-settings.vm | Adds authenticator-specific polling settings (selectors and callbacks). |
| src/identity-server/templates/core/consentor/bankid-signing-consentor/bankid-poller.vm | Adds consentor feature-flag branch to use shared poller/app-launcher and new consentor settings. |
| src/identity-server/templates/core/consentor/bankid-signing-consentor/bankid-polling-settings.vm | Adds consentor-specific polling settings (selectors, callbacks, POST body). |
Comments suppressed due to low confidence (3)
src/identity-server/templates/core/fragments/bankid/bankid-app-launcher.vm:77
- The cancel handler only calls
pollingSettings.onCancelButton()whencancelFunction()returns true. For flows where the cancel element does not submit naturally (e.g. the consentor flow uses atype="button"), a false return fromcancelFunction()will prevent cancellation entirely. Consider always invokingonCancelButton()(and usecancelFunction()only to stop polling), or ensurecancelFunction()always returns true.
src/identity-server/templates/core/fragments/bankid/bankid-poller.vm:90 - In the 201 response handler,
data.message.redirectUrlis accessed without first checking thatdata.messageexists. If the backend returns a 201 payload without amessageobject (e.g. onlystopPolling), this will throw and break polling. Guard withdata.message && data.message.redirectUrlbefore dereferencing.
src/identity-server/templates/core/fragments/bankid/bankid-poller.vm:25 - The header comment says
se.curity.bankid.pollingSettingsmust be defined before this fragment is parsed, but this fragment only definesse.curity.bankid.Pollerand the settings are passed in when constructing the Poller (and in current usage the polling-settings fragment is parsed after this one). Please update the documentation to reflect the actual dependency/order to avoid confusing integrators.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
daniellindau
approved these changes
Mar 12, 2026
src/identity-server/templates/core/fragments/bankid/bankid-app-launcher.vm
Outdated
Show resolved
Hide resolved
src/identity-server/templates/core/fragments/bankid/check-platform-capabilities.js.vm
Outdated
Show resolved
Hide resolved
pmhsfelix
approved these changes
Mar 16, 2026
* Factorize BankID authenticator polling logic Version 2 in shared fragments * Use polling logic Version 2 in BankID signing authenticator, based on a feature flag (bankidConsentorLaunchVersion2). Old polling logic remains the default one.
b778bb3 to
8046e90
Compare
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.
While reviewing, pay attention to all moved templates: this is considered as breaking changes, and may be some of them cannot be simply moved ?