MOSIP-44873: Automated the testcases for OIDC client purpose-based scenarios#1708
Open
BhuvanaShreeBS wants to merge 6 commits into
Open
MOSIP-44873: Automated the testcases for OIDC client purpose-based scenarios#1708BhuvanaShreeBS wants to merge 6 commits into
BhuvanaShreeBS wants to merge 6 commits into
Conversation
Signed-off-by: Bhuvanashree B S <bhuvanashree.b@cyberpwn.com>
Signed-off-by: Bhuvanashree B S <bhuvanashree.b@cyberpwn.com>
…cenarios Signed-off-by: Bhuvanashree B S <bhuvanashree.b@cyberpwn.com>
…cenarios Signed-off-by: Bhuvanashree B S <bhuvanashree.b@cyberpwn.com>
WalkthroughAdds scenario-tag-driven client selection and purpose-specific PAR generation, extends PAR/JWK handling, updates consent-page UI locators and Cucumber steps for purpose-based assertions, and adds multiple client test cases and templates. Changes
sequenceDiagram
participant Test as Test Scenario
participant Base as BaseTest
participant Map as CLIENT_CONFIG_MAP
participant Esignet as EsignetUtil
participant Resource as ResourceBundleLoader
participant Browser as Browser / Driver
Test->>Base: beforeAll() runs
Base->>Map: check scenario source tags
alt Tag matched (mosipid)
Base->>Test: throw SkipException
else proceed
Base->>Map: resolve clientIdKey & clientAssertionPlaceholder
Base->>Esignet: generateParRequestUri(clientIdKey, clientAssertionPlaceholder)
Esignet->>Esignet: select/generate JWK, sign client_assertion
Esignet-->>Base: return PAR request URI
Base->>Resource: fetch prefix texts for assertions
Base->>Browser: replace $REQUEST_URI$ and $CLIENT_ID$ in authorize URL
Browser-->>Test: navigate to resolved authorize URL
end
🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
ui-test/src/main/java/stepdefinitions/InvalidUrlStepDefinition.java (1)
138-164: 🧹 Nitpick | 🔵 TrivialDuplicated literal placeholders across two steps.
userModifiesAuthorizeValueanduserRemovesAuthorizeInUrlboth hardcode"$ID:CreateOIDCClient_all_Valid_Smoke_sid_clientId$"and"$CLIENT_ASSERTION_PAR_JWT$". These same literals also appear as the null-fallback defaults inEsignetUtil.generateParRequestUri(Line 1111) and as the defaults inBaseTest.java(Lines 179-180). If the default client key ever changes, four locations must stay in sync.Extract to package-visible constants (e.g.,
EsignetUtil.DEFAULT_CLIENT_ID_KEY/DEFAULT_CLIENT_ASSERTION_PLACEHOLDER) and reuse them here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/java/stepdefinitions/InvalidUrlStepDefinition.java` around lines 138 - 164, Both step methods hardcode the same placeholder literals that are already used as defaults elsewhere; extract those strings into package-visible constants (e.g., add EsignetUtil.DEFAULT_CLIENT_ID_KEY and EsignetUtil.DEFAULT_CLIENT_ASSERTION_PLACEHOLDER or similar) and replace the literal occurrences in userModifiesAuthorizeValue and userRemovesAuthorizeInUrl with those constants so all places (including EsignetUtil.generateParRequestUri and BaseTest defaults) reference the single source of truth.ui-test/src/main/java/base/BaseTest.java (1)
86-145:⚠️ Potential issue | 🔴 CriticalAdd
@PurposeVerifytoCLIENT_CONFIG_MAP— scenario currently runs against default client instead of purpose-type=verify client.The
@PurposeVerifyscenario (ConsentPage.feature:110–114) is not mapped inCLIENT_CONFIG_MAP, so the loop at lines 177–187 falls through to the default client ($ID:CreateOIDCClient_all_Valid_Smoke_sid_clientId$/$CLIENT_ASSERTION_PAR_JWT$). The scenario's assertions then fail or produce false positives:
validateVerifyPurposeReflectedInUI()asserts UI text derived fromotp.verify_with_idagainst a default "all valid" client, which does not have purpose=verify metadata.verifyDefaultLoginTitleAndSubtitle()asserts hardcoded title "Verify using eSignet" against default client, not a verify-purpose client.- The mosipid skip loop (lines 140–144) won't skip this scenario (inconsistent with other
@Purpose*tags) because@PurposeVerifyis not inCLIENT_CONFIG_MAP.keySet().Proposed fix
CLIENT_CONFIG_MAP.put("@PurposeNone", new String[] { "$ID:CreateOIDCClient_with_purpose_type_none_Smoke_sid_clientId$", "$CLIENT_ASSERTION_PAR_JWT_PURPOSE_NONE$" }); + + CLIENT_CONFIG_MAP.put("@PurposeVerify", + new String[] { "$ID:CreateOIDCClient_with_purpose_type_verify_Smoke_sid_clientId$", + "$CLIENT_ASSERTION_PAR_JWT_PURPOSE_VERIFY$" });This also requires a handler for
$CLIENT_ASSERTION_PAR_JWT_PURPOSE_VERIFY$inEsignetUtil.inputstringKeyWordHandler()(following the pattern of PURPOSE_LOGIN, PURPOSE_LINK, etc.) and the corresponding test-data keys.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/java/base/BaseTest.java` around lines 86 - 145, CLIENT_CONFIG_MAP is missing an entry for "@PurposeVerify", causing the verify scenarios to run against the default client; add a mapping in the static CLIENT_CONFIG_MAP initialization for "@PurposeVerify" with the proper test-data keys (e.g. "$ID:CreateOIDCClient_with_purpose_type_verify_...$" and "$CLIENT_ASSERTION_PAR_JWT_PURPOSE_VERIFY$") so the mosipid skip loop in beforeAll() recognizes the tag and the scenario uses the verify client; also implement the corresponding branch/constant handling in EsignetUtil.inputstringKeyWordHandler() for CLIENT_ASSERTION_PAR_JWT_PURPOSE_VERIFY (mirroring PURPOSE_LOGIN/PURPOSE_LINK handlers) and add the matching test-data keys in your test data files.ui-test/src/main/java/utils/EsignetUtil.java (1)
1098-1123:⚠️ Potential issue | 🟡 MinorParameter validation missing — consider explicit guards or clearer defaults for both parameters.
The silent fallback on null/empty
clientIdKey(lines 1110–1112) is defensive, but current callers (BaseTest.javaandInvalidUrlStepDefinition.java) always pass non-null values, so the fallback is not triggered in practice. However, lacking a guard and logging here makes it harder to catch misconfiguration in future call sites.More pressing:
clientAssertionPlaceholderis put directly into the request body without any null check (line 1123), which would produce"client_assertion": nullif null is passed. While current callers avoid this, an explicit guard would prevent silent failures.Consider adding parameter validation (throw
IllegalArgumentExceptionfor null/empty inputs) or at minimum log when falling back to defaults, to make test configuration issues visible.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/java/utils/EsignetUtil.java` around lines 1098 - 1123, In generateParRequestUri, validate the incoming parameters clientIdKey and clientAssertionPlaceholder instead of silently allowing nulls: add explicit guards at the top of the method that throw IllegalArgumentException (or log and throw) when clientAssertionPlaceholder is null/empty and either throw or clearly log and apply a documented default for clientIdKey; retain the existing fallback behavior that uses AdminTestUtil.replaceIdWithAutogeneratedId for client_id but emit a warning log when that fallback is used so misconfigurations are visible, and ensure any thrown exceptions include the parameter name and context to make failures easy to trace.ui-test/src/main/resources/esignetUI/CreateClientMock/CreateOIDCClient.yml (1)
39-294: 🧹 Nitpick | 🔵 TrivialHigh duplication across 8 near-identical test payloads.
All eight scenarios repeat the same ~20 fields with only
publicKey,purpose_*, andauthContextRefsvarying. Consider leveraging YAML anchors/merge keys (&base/<<: *base) on the common input block, or push shared defaults into the Handlebars template (CreateOIDCClient.hbs) and override only the varying fields per scenario. This will make future field additions (like the newsignup_banner_required,forgot_pwd_link_required,consent_expire_in_mins) a single-point change instead of an 8-place edit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/resources/esignetUI/CreateClientMock/CreateOIDCClient.yml` around lines 39 - 294, The YAML contains eight near-identical test cases (ESignetUI_CreateOIDCClient_with_purpose_type_login_Smoke_sid ... _single_auth_factor_Smoke_sid) duplicating ~20 fields in each "input" block; refactor by extracting the common payload into a base anchor (e.g., &base) and merge it into each scenario using <<: *base while overriding only publicKey, purpose_title, purpose_type, and authContextRefs, or alternatively move the shared fields into the CreateOIDCClient.hbs default/template and pass only the differing keys per test; update the unique scenario entries (publicKey, purpose_*, authContextRefs) to be minimal overrides so future additions like signup_banner_required require a single change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui-test/src/main/java/pages/ConsentPage.java`:
- Around line 344-370: The isLoginWithOtpDisplayed method currently calls
loginWithOtpButton.isDisplayed() directly, which will throw
NoSuchElementException for missing elements; change it to use the existing
helper (e.g., isElementDisplayed(loginWithOtpButton) or
isElementVisible(loginWithOtpButton)) and keep the
getText().trim().startsWith(expectedText) logic so the method returns false
instead of throwing when the element is absent; update the
isLoginWithOtpDisplayed implementation accordingly.
In `@ui-test/src/main/java/stepdefinitions/ConsentStepDefinition.java`:
- Around line 410-414: The assertion messages in verifyTitleNotDisplayed
(method) are hardcoded to "when purpose is none" which is misleading for
scenarios `@NoTitleAndSubTitle` and `@EmptyTitleAndSubTitle`; update the two
Assert.assertFalse calls that call consentPage.isLoginTitleDisplayed() and
consentPage.isLoginSubTitleDisplayed() to use generic failure messages (e.g.,
"Title is displayed" and "Subtitle is displayed") or include dynamic scenario
context if available so the message is accurate for all three scenarios.
- Around line 438-463: Both verifySelectPreferredIdHeaderText and
verifySelectIdTypeHeaderText silently return when authFactors size doesn't match
the expected branch; change them to first assert the authFactors size equals the
expected value (use ClaimsUtil.getAuthFactors() and Assert.assertEquals on size)
so a mismatch fails the step, then compute expectedText and compare with
consentPage.getSelectPreferredIdHeaderText(); also correct the resource key used
in verifySelectIdTypeHeaderText to the single-factor key (replace
otp.login_with_id_multiple with the appropriate single-factor key from
ResourceBundleLoader.get) so the two methods use distinct expected strings.
- Around line 416-420: The method verifyDefaultLoginTitleAndSubtitle currently
uses hardcoded English literals and brittle assertions; replace those literals
with the appropriate resource-bundle lookups (use ResourceBundleLoader.get(...)
or getPrefixText(...) with the same keys used elsewhere for the "verify"
purpose) and assert against consentPage.getLoginTitleText() and
consentPage.getLoginSubTitleText() using equality/contains assertions that
include a clear failure message (e.g., Assert.assertEquals(message, expected,
actual) and Assert.assertTrue(message, actual.contains(expected))). Keep the
method name verifyDefaultLoginTitleAndSubtitle and only change the expected
values to be loaded from the bundle (respect
BaseTestUtil.getThreadLocalLanguage()) and add descriptive assertion messages.
- Around line 369-430: Several no-op step definitions
(userCreateClientIdPurposeLogin, userCreateClientIdPurposeLink,
userCreateClientIdPurposeVerify, userCreateClientIdPurposeNone) and an unused
userCreateClientIdWithNullTitle are either duplicated or dead; replace the four
purpose-specific methods with a single parameterized step like
userCreateClientIdPurpose(String purpose) annotated `@When`("user creates the
client with purpose type {word}") with an empty body, and either remove
userCreateClientIdWithNullTitle or update the feature to reference its exact
step text so it is used (or rename the method to match the feature tag); update
any references in ConsentPage.feature accordingly.
In `@ui-test/src/main/java/utils/EsignetUtil.java`:
- Around line 769-822: The error messages thrown by processClientAssertion are
too generic; update processClientAssertion to include the placeholder/JWK
identifier (e.g. the placeholder strings like
"$CLIENT_ASSERTION_PAR_JWT_NO_TITLE$", "$CLIENT_ASSERTION_PAR_JWT_EMPTY_TITLE$",
"$CLIENT_ASSERTION_PAR_JWT_PURPOSE_LOGIN$", etc. or the corresponding JWK
constant names such as OIDC_JWK_FOR_PAR_NO_TITLE) in the RuntimeException
message so the rethrown exception shows which callsite failed, and make the two
Title callers produce distinct messages (e.g. include "NO_TITLE" vs
"EMPTY_TITLE") to disambiguate failures; ensure all current callers that pass
"PAR error", "Title error", "ACR error", etc. rely on the enhanced message
rather than removing their context.
- Around line 75-96: The block contains dead RSAKey fields
(oidc_JWK_Key_For_PAR_*) and many duplicate triggerESignetKeyGenForPAR*
booleans/getters/setters; replace the booleans and unused RSAKey statics with a
single concurrent Set (e.g., generatedJwkKeys = ConcurrentHashMap.newKeySet()),
remove the unused RSAKey fields (or rename oidc_JWK_Key_For_PAR_Acr_Value to
match OIDC_JWK_FOR_PAR_SINGLE_ACR_VALUE if you must keep it), and add a helper
(e.g., processJWKKey/json placeholder replacer) that takes the JSON, the
placeholder keyword and the JWK name (use the existing OIDC_JWK_FOR_PAR*,
OIDC_JWK_FOR_PAR_PURPOSE_LOGIN, etc.) to generate/cache or fetch the key via
your JWK utility and replace the placeholder; update processClientAssertion to
call that helper instead of calling RSAKey.parse locally so adding new variants
only requires adding a constant and the Set entry.
In `@ui-test/src/main/java/utils/ResourceBundleLoader.java`:
- Around line 78-81: getPrefixText currently calls get(key) and blindly splits
the result, which leaves the "!!MISSING_KEY: <key>!!" marker to flow into later
assertions; update getPrefixText so that after String value = get(key) you check
for the missing-key marker (e.g., value != null &&
value.startsWith("!!MISSING_KEY:")) and immediately throw an informative runtime
exception (IllegalStateException) mentioning the missing key and locale,
otherwise proceed to split on "\\{\\{currentID\\}\\}" and return the trimmed
prefix; reference: getPrefixText and get(key) in ResourceBundleLoader (and
callers like ConsentStepDefinition/isLoginWithOtpDisplayed).
In `@ui-test/src/main/resources/esignetUI/CreateClientMock/CreateOIDCClient.yml`:
- Around line 103-105: The YAML snippet contains hard tabs in the multi-line
scalar for the keys "purpose_title", "purpose_type", and "purpose_subTitle" (and
again at the same block around lines 285-287); replace those tab characters with
spaces so the file consistently uses spaces for indentation throughout—update
the multi-line scalar lines that currently begin with tabs to use the project's
standard number of spaces (e.g., two or four) to normalize formatting and avoid
mixed tab/space diffs.
- Around line 140-142: The purpose fields are inconsistent: when purpose_type is
set to "none" the purpose_title and purpose_subTitle still mention "Login";
update the YAML so purpose_title and purpose_subTitle either reflect a
generic/empty purpose or remove them for the "none" case. Locate the keys
purpose_type, purpose_title and purpose_subTitle in CreateOIDCClient.yml and
either replace "Login using eSignet" and "{{clientName}} is requesting
authentication for login" with neutral text (e.g., a generic title/subtitle) or
delete those two keys so the mock represents a no-purpose client.
In `@ui-test/src/main/resources/featurefiles/ConsentPage.feature`:
- Around line 123-136: Rename and clarify the three conflicting scenarios:
change the `@NoPurpose` scenario's When step from "user creates the client with
purpose type none" to "user creates the client with no purpose" (so it clearly
represents a missing purpose and doesn't reuse the PurposeNone step text), and
change the `@NoTitleAndSubTitle` scenario's title and When step to reference "null
title and subtitle" (e.g., "user creates the client with null title and subtitle
values") so it differs from `@EmptyTitleAndSubTitle`; update the scenario name for
`@NoTitleAndSubTitle` to mention "null" as well and reuse the existing step
definition userCreateClientIdWithNullTitle in ConsentStepDefinition.java for
that scenario. Ensure all three scenario names are unique to avoid duplicate
report rows.
---
Outside diff comments:
In `@ui-test/src/main/java/base/BaseTest.java`:
- Around line 86-145: CLIENT_CONFIG_MAP is missing an entry for
"@PurposeVerify", causing the verify scenarios to run against the default
client; add a mapping in the static CLIENT_CONFIG_MAP initialization for
"@PurposeVerify" with the proper test-data keys (e.g.
"$ID:CreateOIDCClient_with_purpose_type_verify_...$" and
"$CLIENT_ASSERTION_PAR_JWT_PURPOSE_VERIFY$") so the mosipid skip loop in
beforeAll() recognizes the tag and the scenario uses the verify client; also
implement the corresponding branch/constant handling in
EsignetUtil.inputstringKeyWordHandler() for
CLIENT_ASSERTION_PAR_JWT_PURPOSE_VERIFY (mirroring PURPOSE_LOGIN/PURPOSE_LINK
handlers) and add the matching test-data keys in your test data files.
In `@ui-test/src/main/java/stepdefinitions/InvalidUrlStepDefinition.java`:
- Around line 138-164: Both step methods hardcode the same placeholder literals
that are already used as defaults elsewhere; extract those strings into
package-visible constants (e.g., add EsignetUtil.DEFAULT_CLIENT_ID_KEY and
EsignetUtil.DEFAULT_CLIENT_ASSERTION_PLACEHOLDER or similar) and replace the
literal occurrences in userModifiesAuthorizeValue and userRemovesAuthorizeInUrl
with those constants so all places (including EsignetUtil.generateParRequestUri
and BaseTest defaults) reference the single source of truth.
In `@ui-test/src/main/java/utils/EsignetUtil.java`:
- Around line 1098-1123: In generateParRequestUri, validate the incoming
parameters clientIdKey and clientAssertionPlaceholder instead of silently
allowing nulls: add explicit guards at the top of the method that throw
IllegalArgumentException (or log and throw) when clientAssertionPlaceholder is
null/empty and either throw or clearly log and apply a documented default for
clientIdKey; retain the existing fallback behavior that uses
AdminTestUtil.replaceIdWithAutogeneratedId for client_id but emit a warning log
when that fallback is used so misconfigurations are visible, and ensure any
thrown exceptions include the parameter name and context to make failures easy
to trace.
In `@ui-test/src/main/resources/esignetUI/CreateClientMock/CreateOIDCClient.yml`:
- Around line 39-294: The YAML contains eight near-identical test cases
(ESignetUI_CreateOIDCClient_with_purpose_type_login_Smoke_sid ...
_single_auth_factor_Smoke_sid) duplicating ~20 fields in each "input" block;
refactor by extracting the common payload into a base anchor (e.g., &base) and
merge it into each scenario using <<: *base while overriding only publicKey,
purpose_title, purpose_type, and authContextRefs, or alternatively move the
shared fields into the CreateOIDCClient.hbs default/template and pass only the
differing keys per test; update the unique scenario entries (publicKey,
purpose_*, authContextRefs) to be minimal overrides so future additions like
signup_banner_required require a single change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: baaaf2ed-45ec-45fa-b946-75defc30714c
📒 Files selected for processing (10)
ui-test/src/main/java/base/BaseTest.javaui-test/src/main/java/pages/ConsentPage.javaui-test/src/main/java/pages/VideoPreviewPage.javaui-test/src/main/java/stepdefinitions/ConsentStepDefinition.javaui-test/src/main/java/stepdefinitions/InvalidUrlStepDefinition.javaui-test/src/main/java/utils/EsignetUtil.javaui-test/src/main/java/utils/ResourceBundleLoader.javaui-test/src/main/resources/esignetUI/CreateClientMock/CreateOIDCClient.ymlui-test/src/main/resources/esignetUI/CreateClientMock/CreateOIDCClient1.hbsui-test/src/main/resources/featurefiles/ConsentPage.feature
💤 Files with no reviewable changes (1)
- ui-test/src/main/java/pages/VideoPreviewPage.java
Signed-off-by: Bhuvanashree B S <bhuvanashree.b@cyberpwn.com>
damodarguru
approved these changes
Apr 22, 2026
mohanachandran-s
approved these changes
Apr 22, 2026
zesu22
approved these changes
Apr 22, 2026
Contributor
zesu22
left a comment
zesu22
left a comment
There was a problem hiding this comment.
approving because it is already approved by @mohanachandran-s
Signed-off-by: BhuvanaShreeBS <bhuvanashree.b@cyberpwn.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
ui-test/src/main/java/stepdefinitions/ConsentStepDefinition.java (1)
458-465:⚠️ Potential issue | 🟡 MinorFix the single-auth-factor assertion context.
Line 463 expects exactly one auth factor but reports “Expected multiple auth factors”. Also reusing
otp.login_with_id_multipleon Line 464 makes the single-factor scenario assert the multiple-factor copy.🛠️ Proposed fix
- Assert.assertTrue(authFactors.size() == 1, "Expected multiple auth factors, got " + authFactors.size()); - String expectedText = ResourceBundleLoader.get("otp.login_with_id_multiple"); + Assert.assertEquals(authFactors.size(), 1, "Expected exactly one auth factor, got " + authFactors.size()); + String expectedText = ResourceBundleLoader.get("otp.login_with_id_single");Use the actual single-factor resource key if it differs from
otp.login_with_id_single.#!/bin/bash # Verify the available resource-bundle keys for single-vs-multiple ID header text. rg -n -C2 'otp\.login_with_id|login_with_id_single|login_with_id_multiple'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui-test/src/main/java/stepdefinitions/ConsentStepDefinition.java` around lines 458 - 465, The assertion in verifySelectIdTypeHeaderText incorrectly expects a single auth factor but reports "Expected multiple auth factors" and uses the multiple-factor resource key; update the assertion message and resource lookup to match the single-factor case: use ClaimsUtil.getAuthFactors() to assert size == 1 with a message like "Expected single auth factor, got X", and change ResourceBundleLoader.get("otp.login_with_id_multiple") to the single-key (e.g. "otp.login_with_id_single") so consentPage.getSelectPreferredIdHeaderText() is compared against the correct single-factor string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui-test/src/main/java/utils/EsignetUtil.java`:
- Around line 855-862: processJWKKey currently calls
generatedJwkKeys.add(jwkKeyName) before generation, which can let another thread
read a not-yet-generated key; fix by making the cache generation atomic —
replace the boolean set usage with an atomic compute-if-absent pattern so
generation and insertion happen under a single atomic operation (e.g., use a
ConcurrentHashMap<String,String> cache and call computeIfAbsent(jwkKeyName, k ->
JWKKeyUtil.generateAndCacheJWKKey(k)) or otherwise synchronize per-key) and then
call JWKKeyUtil.getJWKKey(jwkKeyName) or use the computed value; update
processJWKKey to use that atomic cache strategy instead of
generatedJwkKeys.add(...) / getJWKKey(...) branching to avoid the race.
In `@ui-test/src/main/resources/esignetUI/CreateClientMock/CreateOIDCClient.yml`:
- Around line 184-217: The test
ESignetUI_CreateOIDCClient_with_purpose_title_and_subtitle_null_Smoke_sid uses
quoted Handlebars placeholders in the CreateOIDCClient template so nulls render
as empty strings; create a new template (e.g., CreateOIDCClient_NullPurpose)
that uses unquoted placeholders for purpose_title and purpose_subTitle (so they
render as JSON null) and change the test's inputTemplate from CreateOIDCClient
to that new template; ensure the new template places {{purpose_title}} and
{{purpose_subTitle}} outside quotes (and keep the rest of the payload the same)
so the test actually validates JSON null behavior.
In `@ui-test/src/main/resources/featurefiles/ConsentPage.feature`:
- Around line 105-126: The scenarios tagged `@PurposeLink`, `@PurposeVerify`,
`@PurposeNone` and `@NoPurpose` are asserting/clicking consent-login elements
without capturing the current authorize URL, which can lead to flaky results;
update each Scenario (the ones using steps "When user creates the client..." and
subsequent Then/And steps) to invoke the existing step "Given user captures the
authorize url" immediately after the Scenario starts (i.e., before any
assertions or clicks on consent-login elements) so that consent-page actions use
the fresh authorize URL and avoid stale-state failures.
---
Duplicate comments:
In `@ui-test/src/main/java/stepdefinitions/ConsentStepDefinition.java`:
- Around line 458-465: The assertion in verifySelectIdTypeHeaderText incorrectly
expects a single auth factor but reports "Expected multiple auth factors" and
uses the multiple-factor resource key; update the assertion message and resource
lookup to match the single-factor case: use ClaimsUtil.getAuthFactors() to
assert size == 1 with a message like "Expected single auth factor, got X", and
change ResourceBundleLoader.get("otp.login_with_id_multiple") to the single-key
(e.g. "otp.login_with_id_single") so
consentPage.getSelectPreferredIdHeaderText() is compared against the correct
single-factor string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 220e4108-6627-4fe0-a085-fc32142c9946
📒 Files selected for processing (6)
ui-test/src/main/java/pages/ConsentPage.javaui-test/src/main/java/stepdefinitions/ConsentStepDefinition.javaui-test/src/main/java/utils/EsignetUtil.javaui-test/src/main/java/utils/ResourceBundleLoader.javaui-test/src/main/resources/esignetUI/CreateClientMock/CreateOIDCClient.ymlui-test/src/main/resources/featurefiles/ConsentPage.feature
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
4 participants
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.
Automated the testcases for OIDC client purpose-based scenarios
Summary by CodeRabbit
Tests
New Features
UI