Conversation
cas-server compiles against iam-commons, which carries IAM's administration model and its pac4j helpers. Consuming the authentication contract should not require the management module. iam-auth-contract holds the contract alone: the DTO an authentication server exchanges with IAM, and the paths it calls. It depends neither on pac4j nor on iam-commons, so it cannot grow into a second home for domain logic. LoginRequestDto moves there, and the CAS_* path constants leave RestApi for AuthContractApi. The URL values keep their historical /cas segment — changing them would break the REST contract with deployed servers. Only the constant names stop naming a product, since the contract is no longer specific to Apereo CAS. The openapi generator's import mapping in iam-client follows the move.
The webflow decides today which organisations an email may authenticate against, and by which identity provider. That decision is domain knowledge: it reads users, customers and providers, and it carries a security policy — the response must not reveal whether an account exists. Keeping it in the authentication server forces that server to hold IdentityProviderHelper, the full provider list, and the keystores that come with it. CasService.resolveHrdEntries answers the question in one call. Two signals feed it and both are needed: provider patterns cover an email with no account yet, where a federated provider will provision on first login; existing accounts cover an internal account whose address matches no pattern. An internal provider is kept only when an account really exists in its organisation, which drops the catch-all pattern false positives. HrdEntryDto carries the user status too, so the caller can refuse a disabled account without a second round trip — the webflow needs one today. CasServiceHrdTest states the truth table the webflow satisfies now, case by case, and names the test each one comes from. It is what will show the behaviour did not change once the decision moves.
iam-auth-contract is only worth having if it stays empty of everything but the contract. Nothing said so out loud, so a single added dependency would have turned consuming it back into consuming IAM whole. maven-enforcer now bans iam-commons, iam-client, pac4j, Apereo CAS and Spring Data from the module. Verified by adding pac4j-core and watching the build fail; the iam-commons case never even reaches the rule, since Maven rejects the dependency cycle first. The ArchUnit rules alongside it are the readable double of that list — they name the intent where the pom only names artifacts. The two rules that would bite hardest are missing on purpose: an authentication server should reach IAM only through this contract, and CasController should return only contract types rather than the administration DTO it returns today. Neither passes yet; both belong to the branch that reconnects cas-server and auth-server.
The authentication server shows the password constraints and IAM checks them, but each reads its own configuration file. Nothing keeps the two in step, so a drift shows up as a form that accepts a password IAM then rejects — with no way for the user to tell why. CasService.getPasswordPolicy publishes what IAM enforces: minimum length, profile, password history depth, and the constraint labels flattened in configuration order — defaults first, special characters interleaved, custom constraints last. That order is what the login page displays, so the test pins it. The endpoint only exposes the policy; making cas-server read it instead of its own copy belongs to the reconnection branch, since InitPasswordConstraintsConfiguration runs as a ServletContextInitializer, before the HTTP layer it would need is up.
…lished The HRD and password policy endpoints existed on the server but not in the specification iam-client generates from, so CasApi did not know them and no authentication server could call them. Adding them by hand was possible only because someone knew to look — the pom still carries its "TODO: Swagger file should be generated during build". Both operations are now declared, and the generator maps them onto the contract classes rather than generating copies: CasApi returns List<HrdEntryDto> and PasswordPolicyDto from iam-auth-contract directly. CasContractPublicationTest closes the gap that let this happen. It reads the operationId of every @operation on CasController and requires each to appear in the specification. Verified by renaming one in the yaml and watching the test fail — the first version compared with contains() and passed on the rename, since the original was a prefix of it; it now compares whole identifiers.
…e webflow IamSurrogateAuthenticationService asks IAM for every subrogation the super-user holds, then looks through them for one matching all four values. The whole list travels for a single yes-or-no, and the rule itself — which four values must match — lives in the authentication server. CasService.validateSubrogation answers directly and returns both resolved user ids. A refusal is a NotFoundException, never an empty response, so a caller cannot read "not allowed" as "nothing to report". One deliberate change of behaviour: an expired subrogation is now refused. The current filter never looks at the date, trusting the Mongo TTL index on Subrogation.date to have purged the entry — but that index runs once a minute and some deployments disable it, leaving a window where an expired subrogation still works. The test states this case explicitly rather than letting it pass as a silent tightening.
UserPrincipalResolver assembles the authentication attributes itself, which forces the authentication server to know some sixty attribute names from CommonConstants and how each one derives from the user model. Adding an attribute in IAM meant editing the authentication server too. CasService now assembles that table. buildPrincipalAttributes resolves the user — including the surrogate case and its extra embedded blocks — and toPrincipalAttributes performs the mapping alone, so the mapping can be verified without standing up the whole resolution. Every value is a string, booleans and dates included. That is not a loss: it is already the form applications receive, since AuthUserDto.buildFromAttributes reads them back with Boolean.parseBoolean((String) value) and OffsetDateTime.parse((String) value), and its parseJson returns null outright for anything that is not a String. Composite values are serialised with the same JsonUtils that CasJsonWrapper.toString() used, so the transmitted string is unchanged — which is what makes the move invisible to consumers. A null value is omitted rather than carried as "null": the reader switches on the keys present, where an absent key and a null one mean the same thing.
The Home Realm Discovery unit cases checked the rule against providers built for it, and passed. Replayed in a real MongoDB against the data set shipped with the product, the same rule diverges from the webflow on two counts. An internal provider was added alongside a delegation for the same customer, because the account-driven branch kept it without checking that one of its patterns covered the address. A federated account was therefore also offered its customer's password, and the ordering between the two entries, made on the customer code they share, was indeterminate. An unknown address on a domain served by an internal provider resolved to nothing, the filter dropping internal providers that have no account. The response then told a known address apart from an unknown one before any password was entered, where ListCustomersAction routes both alike and lets the failure happen at authentication, in a generic form. The two sources do not carry the same weight, contrary to what the union did: existing accounts have the final say, patterns only serve in their absence. Within a customer, the provider kept is the first whose pattern matches, taken in identifier order — the very order by which ProvidersService puts the internal provider ahead of the delegations. The helper that built an internal provider with no pattern at all described a customer neither of the webflow's two paths can reach; that is what made these tests complacent. Two cases change meaning and say so: a provider without an account stays offered, and so does a disabled one — neither ProvidersService nor IdentityProviderHelper looks at "enabled", and fixing it here alone would move IAM away from the behaviour it has to reproduce. The data set is an extract of the development database, stripped of keystores, metadata and client secrets: HRD reads none of them.
The surrounding code documents itself in English, down to the webflow comment this work leans on the most — "To avoid account existence disclosure, unknown users are silently ignored." Everything added for the decoupling was written in French, which splits the language of a contract two authentication servers are meant to read. Javadoc, inline comments and test display names are translated. The constraint labels of the password policy test follow, being invented values rather than reference data. The HRD javadoc on CasController was orphaned: a second javadoc block immediately followed it, so it documented no method at all. It now sits on resolveHrd.
The delegated providers cache was loaded eagerly at startup. When IAM was down CAS failed to boot, adding a start-up ordering dependency that no step of the decoupling intended. afterPropertiesSet now starts with an empty cache and tolerates IAM being unreachable, logging the failure instead of aborting. The scheduled reload fills the cache as soon as IAM answers. CAS can boot on its own; the providers appear once IAM is reachable.
…email Creating a user in IAM depended on CAS being up: the activation email that lets a new user set a password was built and sent by CAS, so IAM had to reach CAS to finish a creation, and a CAS outage blocked it. Password reset lived in CAS only because that is where the reset screens are, not by design. IAM now owns the email. A dedicated UserEmailService composes the message from IAM's own templates and sends it; CAS is left with a single responsibility, to return the reset link. User creation no longer hard-depends on CAS. The email configuration moves out of the reserved iam prefix into its own strict block, and user management keeps its declarative transactions so a failed send never leaves a half-created user.
…dpoint CAS delegated the subrogation decision to the IAM through a coarse getSubrogations call whose result it re-filtered locally, case-sensitively. Call the dedicated validateSubrogation endpoint instead: the IAM owns the whole decision (accepted status, expiry, both users resolvable) and answers authorized/refused. CAS stays fail-closed on any error.
…point The login dispatch decided the realm itself: it listed users by email, matched identity-provider patterns locally (case-sensitively, first match wins) and fetched customers separately. Ask the IAM resolveHrd endpoint instead, which returns one entry per customer with its provider, internality and user status already resolved. ListCustomersAction no longer needs the providers cache nor the local helper; DispatcherAction keeps the providers cache only to look up the pac4j client used for the external redirection.
The static cas_secret_token grants the internal "casuser" service account. Until now the secret alone was enough: any mTLS client that knew the string became casuser with the ROLE_CAS_* authorities, because the certificate that had just been verified was never consulted when the secret was accepted. The external provider now attaches the certificate-resolved context to the pre-authenticated token, and IamUserAuthentificationService accepts the secret only when that context carries ROLE_CAS_LOGIN, the marker of the CAS application context. Knowing the secret is no longer sufficient; the caller must also present the real cas-server certificate. Legitimate usage is unchanged.
Logout revoked the IAM token through the CAS ticket-granting ticket. When the TGT was already gone the revocation was silently skipped, and an expired subrogation with a live token could raise a NullPointerException, so a session could survive its own logout. Revocation is extracted into revokeIamSession, called directly on the token rather than through the ticket. It always runs, logs a warning when the ticket is already absent instead of failing silently, and no longer throws when there is nothing left to revoke. Logout now reliably kills the token.
…case-insensitively When a user returns from an external identity provider, the resolver already required the identifier returned by the provider to match the one announced in the session. Two gaps remained: the comparison was case-sensitive, so the same address in a different case was rejected, and a blank returned identifier would either slip through or raise a NullPointerException instead of a clean refusal. The check now refuses whenever either identifier is blank and compares them case-insensitively. A federated login is accepted only when the provider returns exactly the announced identity, in any case.
CAS decided password expiry itself from the expiration date the IAM returned: the last authentication decision it still owned. The IAM now computes the verdict at login and returns it on UserDto; CAS only reads it and raises its exception. Behaviour is unchanged, including the implicit "no expiration date means expired" rule, now stated where the rule lives.
The IAM, which owns the password, now applies the composition policy (pattern and name-occurrence checks) and rejects a non-compliant password itself, instead of relying on the authentication server to be the one that judges it.
…uration Carry the password policy pattern in the IAM deployment configuration so the rule the IAM enforces matches the one that was applied on the CAS side.
…d change Only a user linked to an internal identity provider may change a password. The IAM owns the providers, so it now runs this check against the source rather than trusting CAS and its 60s-refreshed copy. The check stays on the CAS side too, to drive the screen feedback.
The IAM now owns the password rules, so it, not the authentication server, decides a refusal. It attaches a stable key to each refusal (policy not matched, name occurrence, provider missing or external). The authentication server stops re-judging the policy locally and instead translates the two screen-facing keys back into the password page's own exceptions.
…token The last decision the authentication server still made on its own was assembling the principal: it fetched the user, computed the OTP flag from its local copy of the providers, resolved the super user and laid out some thirty attributes. It now asks the IAM, which owns that data, through buildPrincipalAttributes. To keep this iso-behaviour the endpoint returns a typed response rather than a map of strings: scalars stay typed, the two enumerations travel as their name and the five composite attributes travel as their already-serialized JSON, which the authentication server republishes verbatim through a small RawJson wrapper. The token bytes are therefore unchanged while the authentication server stops depending on the administration model to shape the principal. The x509, delegation and subrogation identity resolution stays where the credential is.
…rofile The reconnected endpoints (resolveHrd, validateSubrogation, buildPrincipalAttributes, getPasswordPolicy) are secured with new roles. The effective authorities of an external call are the intersection of the certificate context roles and the user profile roles, so both the CAS context (cas_context) and the CAS profile (casuser -> cas_group -> cas_profile) must carry them. Without them the authentication server got a 403 the moment it started calling the endpoints. Add the roles to both in the base seed, and a migration for existing databases.
… all-args creator The request DTO carried @AllArgsConstructor next to @NoArgsConstructor. Jackson 3 picked the all-args constructor as a properties-based creator, and rejected a request whose primitive apiContext was absent (Cannot map null into boolean), which is exactly what the authentication server sends on a browser login. Drop the unused all-args constructor so deserialization goes through the no-args constructor and setters, leaving an absent apiContext at its false default. Add a test that pins this down.
On the delegated authentication return path, the authentication server no longer reads the e-mail/identifier out of the IdP profile nor checks the returned e-mail: it forwards the raw IdP identity (provider id, principal id, attributes) and the IAM applies the provider's mailAttribute/identifierAttribute mapping (falling back to the principal id) and enforces that the returned e-mail matches the one the user asked to sign in with. OIDC and SAML share this return path, so both are decoupled at once. Behaviour is preserved: a missing mapped attribute or a mismatched e-mail still refuses the login.
Cleanup of symbols orphaned once the identity rules moved to the IAM. No behaviour change: every removed element had no remaining production caller (verified repo-wide, including Spring bean wiring, webflow configurers and message properties). Removed: - CasJsonWrapper (+ its test): superseded by RawJson for the principal JSON attributes. - UserPrincipalAttributes: its two attribute names are served by CommonConstants. - IamPasswordManagementService: the injected centralAuthenticationService and ticketRegistry were never used (also dropped from the passwordChangeService bean). - UserPrincipalResolver.DEFAULT_PROVIDER constant, DispatcherAction.TRANSITION_SELECT_CUSTOMER and its now-unreachable webflow transition. - Orphan config key login.url and a stale filename comment. - Dead test constant PASSWORD_CONTAINS_DICTIONARY_INSENSITIVE. - Obsolete cas-server-application-dev.yml.bak.
…D decoupling The GET /iam/v1/cas/users?email endpoint and its whole chain were orphaned when home-realm discovery moved to resolveHrd: ListCustomersAction was its only client and it no longer calls it. Removes the endpoint (CasController), CasService.getUsersByEmail, UserService.findUsersByEmail, the cas_getUsersByEmail swagger operation (regenerating the client without it), and the two @disabled tests plus helper that exercised the old flow. The repository method findAllByEmailIgnoreCase is kept: it is still used by CasService for provisioning.
…brogation decoupling The GET /iam/v1/cas/subrogations endpoint became dead once CAS switched to validateSubrogation: IamSurrogateAuthenticationService no longer calls it. Removes the endpoint (CasController), the CasService.getSubrogationsBySuperUser and its convertFromSubrogationToDto helper, the now-unused SubrogationRepository.findBySuperUserAndSuperUserCustomerId, the swagger operation (regenerating the client without it), and the orphaned subrogationService field / imports. Kept: SUBROGATIONS_PATH and ROLE_CAS_SUBROGATIONS (still referenced by the delete-subrogation tests and the role catalogue).
login.url is consumed cross-module by Pac4jClientBuilder (iam-commons) via ${login.url} to build
the delegated clients' callback URL and the SAML SP entityId; the dead-code sweep only grepped
cas-server and missed it. Removing it broke SAML/OIDC delegation (invalid SP entityId / missing
callback). Restored with a comment documenting the cross-module usage.
UserLoginModel is only a JSON (de)serialization DTO for the password-reset token; it is never put in a Set/Map nor compared, so equals()/hashCode() are dead. toString() is kept (used in logs).
…the dead-code cleanup Formatting-only (import ordering, line wrapping) plus removal of imports left unused by the earlier dead-code removals: SubrogationDto (CasService), Optional (CasController), CommonConstants/UserDto/ AuthUserDto/List (CasServiceIntegrationTest). No behaviour change.
casApi.login() returns a non-null, ENABLED, NOMINATIVE user or throws: the IAM's findUserByEmailAndCustomerId rejects an absent user (NotFound), a non-nominative one (InvalidAuthentication) and a bad status (InvalidFormat), and the throttling resets a blocked-under-limit user to ENABLED. So the handler's 'user == null' else, the 'status == ENABLED && type == NOMINATIVE' gate and its else were unreachable; the failure cases stay mapped identically by the catch blocks. Flattened + removed 4 now-unused imports.
…SubrogationAction The four flowScope entries userEmail/userCustomerId/superUserEmail/superUserCustomerId (commented 'CAS 7 / OIDC compatibility as used in v9.0') are never read - not in Java main, not in the templates (which read surrogateEmail/surrogateCustomerCode/surrogateCustomerName) - and duplicate the FLOW_* keys written just above. Dropped them and the matching test assertions.
…ordManagementService Password composition validation moved to the IAM (CasService.checkPasswordPolicy); the injected PasswordConfiguration was left used only by a LOGGER.debug. Removed the field, constructor param, its wiring in the passwordChangeService bean, and the test setup. No behaviour change (the composition rules are enforced IAM-side; the CAS UI still displays constraints via InitPasswordConstraintsConfiguration).
… of 2-3 times isUserDisabled and dispatchUser each re-called casApi.resolveHrd for the same user, so a login hit resolveHrd twice and a subrogation hit it twice for the super-user. resolveHrd is an idempotent GET: resolve the HrdEntryDto once per user and pass it to the disabled check and the dispatch. Behaviour identical (same entries, same routing); one fewer IAM round-trip per login.
The authentication server used to resolve the certificate identity provider itself: findAllProvidersByUserIdentifier on its loaded provider copy, filtered to the CERTIFICAT protocol, with the "exactly one provider (no multi-domain)" rule enforced locally. That identity rule now lives in IAM. - New IAM endpoint GET /iam/v1/cas/certificate?userIdentifier=... backed by CasService.resolveCertificateProvider: same helper, same provider set (identityProviderService.getAll mirrors the server's getAll(null, ...)), same CERTIFICAT filter and single-provider enforcement - only relocated, so the resolution is behaviour-identical. It returns the provider id + customer id as an HrdEntryDto, and refuses (404) when none or several match. - UserPrincipalResolver keeps what is genuinely the server's job (extracting the e-mail/identifier from the certificate and applying the default domain) and now calls casApi.resolveCertificateProvider; a refusal still stops the flow with a NullPrincipal, exactly as before. Iso-behaviour. The two per-cause WARN logs (none / several) move to IAM where the rule now lives; the server logs one refusal at WARN. WARNING: this path cannot be exercised in the dev environment (no client certificates). Live validation with real certificates is required before merge.
Cover CasService.resolveCertificateProvider: the single-CERTIFICAT match is returned as an HrdEntryDto, non-CERTIFICAT protocols are filtered out, and the resolution is refused (NotFoundException) when none or several providers match - the multi-domain guard that used to live in the authentication server. Case-insensitive pattern matching stays covered by IdentityProviderHelper (the shared, unchanged resolver this delegates to).
…ed mapping
The delegation decoupling (377338f79) moved the IdP-attribute mapping and its
checks from the authentication server to the IAM, but three tests still asserted
the old local decision - a NullPrincipal ("nobody") when the mapped mail/identifier
attribute was empty. They passed on develop and broke on this branch.
They now reflect the decoupled flow: the server forwards the raw profile and the
IAM refuses the login (missing mapped attribute); a refusal maps to a null
principal. The empty-attribute rejection itself is covered where the rule now
lives, in CasServiceDelegatedIdentityTest.refusesWhenAMappedAttributeIsMissing.
…contract The handler simplification (ca9ea9493) dropped the dead null/status re-checks: the IAM login now returns an ENABLED, NOMINATIVE user or throws. Three tests still drove the removed local behaviour by making casApi.login return null / DISABLED / BLOCKED - states the IAM contract never returns. They passed on develop and broke on this branch. They now feed the exceptions the IAM actually raises and assert the handler's mapping: - testNoUser: unknown user -> NotFoundException -> PreventedException (no null). - testUserDisabled: refused status -> InvalidFormatException -> AccountException, which also fills the previously untested InvalidFormat mapping. - testUserCannotLogin removed: a blocked account is a TooManyRequestsException, already covered identically by testUserLockedAccount.
…y (lot A/B) Class-by-class pass over cas-server, behaviour-preserving unless stated: - InitContextConfiguration: one helper for logo and favicon; a missing favicon no longer aborts the boot (it was a warning for the logo, an exception for the favicon). - InitPasswordConstraintsConfiguration: drop the null checks on an @Autowired field and the guard already enforced by the preceding throw; stop calling toString() inside parameterised debug logs; one putConstraints helper. - CheckMfaTokenAction: a token whose ticket is missing or invalid is refused instead of passing through the expiration check (security fix). - CustomCorsProcessor: @OverRide on handleInternal; derive the IdP origin with java.net.URI instead of indexOf("/", 9). - CustomOidcCasClientRedirectActionBuilder: url-encode the forwarded subrogation/login parameters (an e-mail carrying '+' was corrupted). - CustomDelegatedAuthenticationClientLogoutAction: no more unguarded get() on the provider lookup. - DispatcherAction: the disabled-surrogate log named the super-user e-mail. - CustomSurrogateInitialAuthenticationAction: license header, accurate log. - AppConfig: copy the shared RestClient.Builder before adding the X-Origin interceptor; drop a needless @SneakyThrows; lambda for the MFA strategy; the two identical surrogate/x509 principal-resolver aliases become one bean with two names. - WebConfig: one buildAdapter() for the duplicated security adapter wiring. - WebflowConfig: pmTicketFactory is a plain helper (it was never injected); TRANSITION_ID_ERROR instead of "error"; named MFA configurer order. - Records for X509AttributeMapping and CustomerModel; BeanUtils.copyProperties for Pac4jClientIdentityProviderDto (a hand-written copy dropped new fields). - LoginPwdAuthenticationHandler: the request is built and logged in one method. - VitamLoginWebflowConfigurer: one bindAndValidate() for the three submits. - CustomRequestHeaderX509CertificateExtractor: shared PEM decoding for the Nginx and Apache header formats. - Named constants for the remaining magic numbers (TGT timeout, reload interval, wording threshold, hidden URL tail, flow key); commons-lang 2 -> commons-lang 3; Constants is final; dead throws clauses and imports removed.
… instead of re-building them (lot C) Two beans re-created, argument for argument, what CAS already provides and had to be maintained at every CAS upgrade. Both defaults are @ConditionalOnMissingBean in CAS 7.0.10.1 (checked in the jars), so dropping the override lets CAS wire its own: - AppConfig.delegatedClientAuthenticationConfigurationContext (33 @qualifier): CAS's DelegatedAuthenticationWebflowConfiguration builds the same context and already picks up the two beans this module overrides (delegatedIdentityProviders and delegatedAuthenticationCredentialExtractor). - WebflowConfig.initialAuthenticationAttemptWebflowEventResolver: it re-registered CAS's twelve delegates by hand only to substitute X509CasDelegatingWebflowEventResolver, whose sole effect was one error log on a failed mandatory X509 login. The class is removed too. WebConfig.corsHttpWebRequestConfigurationSource is deliberately kept: CAS's default is conditional while corsFilter needs the source unconditionally. Verified live: CAS boots on the defaults and the X509 non-regression scenario - which now runs through CAS's own event resolver - still passes. OIDC/SAML delegated login and a CORS pre-flight are to be re-tested once the test IdP is provisioned again.
…nalise the service account (lot D) - One shared ObjectMapper on Utils (toJson/fromJson) replaces the three private "new ObjectMapper()" of IamPasswordManagementService, ResetPasswordController and I18NSendPasswordResetInstructionsAction. Same default configuration, so the password-reset token is serialised exactly as before; ResetPasswordController no longer needs the mapper injected. - IamApiDecorator's service-account e-mail is no longer hard-coded: it comes from vitamui.cas.service-account, defaulting to the previous admin@change-it.fr. Verified live: CAS boots and the X509 non-regression scenario (which calls the IAM through the decorated client under the service-account context) still passes.
|
Important Review skippedToo many files! This PR contains 120 files, which is 20 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (120)
You can disable this status message by setting the 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 |
hyahyaoui
changed the title
hyahyaoui
marked this pull request as draft
September 8, 2026 12:24
Collaborator
|
New Issues (45)Checkmarx found the following issues in this Pull Request
Fixed Issues (10)Great job! The following issues were fixed in this Pull Request
Use @Checkmarx to interact with Checkmarx PR Assistant. |
Resolve 5 conflicts, keeping the IAM-decoupling design (Story #16712) while adopting develop's changes: - UserEmailService / UserEmailServiceTest: keep IAM sending the initialization e-mail itself (reverse IAM->CAS call already cut), adopt develop's class rename VitamuiRestClientFactory -> RestClientFactory (Story #16471). - CasController: keep the contract path AuthContractApi.LOGOUT_PATH and the removal of the dead getSubrogations GET endpoint; develop's re-added copy is dropped (no caller remains). - UserPrincipalResolver: drop develop's inline "Invalid user from Idp" check; the same verification now lives in IAM CasService, case-insensitive and fail-closed, which already covers develop's Bug #16751 fix. - AppConfig: keep cloning the shared RestClient.Builder so the X-Origin interceptor does not leak globally, using the renamed IamApiClientsFactory. Validated: full CAS suite 103/103, IAM UserEmailService/CasController/CasService tests 29/29, whole reactor compiles.
Integrate develop's CAS 7.0.10.1 -> 7.3.8 / Spring Boot upgrade (Story #16347) and the dependency bump (Story #16790). Conflicts resolved keeping the IAM-decoupling design while adopting the CAS 7.3 API shape: - DynamicTicketGrantingTicketFactory: adopt CAS 7.3 produceTicket signature (no Class<T>, no cast), keep our named timeout constant. - UserPrincipalResolver: drop getAttributeRepository (removed from the CAS 7.3 PrincipalResolver) and develop's local attribute building (IAM owns it). - WebConfig: adopt the CAS 7.3 security adapter (managementServerProperties, webProperties, configureHttpSecurity(http, applicationContext)); drop the buildAdapter helper. - AppConfig: adopt develop's non-bean iamRestClientCustomizer (supersedes our builder clone); keep dropping delegatedClientAuthenticationConfigurationContext (the CAS default already picks up our overridden beans). - IamPasswordManagementService: keep the password policy in the IAM (no local passwordConfiguration / validation). CAS 7.3 API adaptations that do not compile yet are handled in a dedicated follow-up commit.
Follow-up to the develop merge, isolating the CAS 7.3 API drift the merge left: - AppConfig: import TicketRegistry and ConfigurableApplicationContext, now used by the CAS 7.3 defaultAccessTokenFactory and iamSurrogateAuthenticationService beans. - WebflowConfig: keep pmTicketFactory a public @bean as CAS 7.3 expects (the merge had silently kept our private clean-code variant, which would drop the transient-session-ticket factory the password-reset flow needs). - CasBeanOverridesTest: drop delegatedClientAuthenticationConfigurationContext and initialAuthenticationAttemptWebflowEventResolver from the pinned override surface - the decoupling intentionally stopped overriding them (the CAS defaults now suffice), and document why. - IamSurrogateAuthenticationServiceTest: call getImpersonationAccounts with the CAS 7.3 two-argument signature (username, Optional<Service>). - IamPasswordManagementServiceTest: build the service with CasConfigurationProperties (CAS 7.3 constructor) instead of PasswordManagementProperties. - TerminateApiSessionActionTest: align the fixture with the CAS 7.3 action constructor (no ApplicationContext, add the web application service factory). Validated: cas-server suite 112/112, IAM unit tests green (only the MongoDB testcontainer integration tests fail, for an unrelated kernel/MongoDB 8.0 issue).
hyahyaoui
force-pushed
the
story_16712_cas_iam_decoupling
branch
from
September 9, 2026 09:22
4afab56 to
137d0dd
Compare
…nto v10.0 The CAS decoupling roles (ROLE_CAS_HRD, ROLE_CAS_SUBROGATION_VALIDATE, ROLE_CAS_PRINCIPAL_ATTRIBUTES, ROLE_CAS_PASSWORD_POLICY) had been hardcoded into the 1.0.0 base seed (cas_context and cas_profile) on top of already being granted by the v10.0/0-03_add_cas_decoupling_roles.js migration - duplicating the same grant through two mechanisms and breaking the versioning discipline: migration scripts belong to the version that introduces them, not to the base seed, and editing an old seed file has no effect on an environment that already ran it (the changelog skips scripts by filename, not by content). Remove the duplicated lines from the 1.0.0 seed; 0-03 remains the single source of truth for those four roles. Also add 0-05_add_cas_customers_role.js: CAS calls GET /iam/v1/cas/customers (ROLE_CAS_CUSTOMERS) while building a subrogation, but that role was only ever granted by the v7.1 migrations (58_/59_update_*_cas_customer_role.js). An environment seeded from 1.0.0 without replaying v7.1 never receives it, causing a 403 on subrogation. New migration in v10.0, consistent with the same versioning rule.
hyahyaoui
marked this pull request as ready for review
September 9, 2026 15:25
Regzox
approved these changes
Sep 10, 2026
Regzox
left a comment
Regzox
left a comment
Contributor
There was a problem hiding this comment.
La PR retraduit en français quantité de javadoc existante (CasController, UserService, WebConfig, ExternalApiAuthenticationProvider, TerminateApiSessionAction, CustomCorsProcessor…), à rebours de la
convention du dépôt
| ); | ||
| } | ||
|
|
||
| /** |
Contributor
There was a problem hiding this comment.
Double bloc de javadoc à corriger.
| ); | ||
| } | ||
|
|
||
| public List<HrdEntryDto> resolveHrdEntries(final String email) { |
Contributor
There was a problem hiding this comment.
Le commentaire en trop documente cette méthode ?
| @RequestParam(value = "username", defaultValue = "") final String username, | ||
| @RequestParam(value = "firstname", defaultValue = "") final String firstname, | ||
| @RequestParam(value = "lastname", defaultValue = "") final String lastname, | ||
| @GetMapping("/passwordResetUrl") |
Contributor
There was a problem hiding this comment.
Fonctionne mais semble être exposé sans restrictions. Voir si public à travers le rpx, voir si on peut trouver une mécanisme ne permettant pas une utilisation open bar.
| @GetMapping(value = AuthContractApi.PASSWORD_POLICY_PATH) | ||
| @Operation(operationId = "cas_getPasswordPolicy", summary = "Get the password policy enforced by IAM") | ||
| @Secured(ServicesData.ROLE_CAS_PASSWORD_POLICY) | ||
| public PasswordPolicyDto getPasswordPolicy() { |
| return customerService.getAllById(customerIds); | ||
| } | ||
|
|
||
| /** |
Contributor
There was a problem hiding this comment.
Petite bidouilles au niveau de la doc.
CasService.buildPrincipalAttributes
/**
* Assembles the full authentication attribute set of a user, as the token will carry it.
*
* What moved here is the derivation, not the naming: the authentication server still writes the
* attribute keys itself, but it no longer decides what each one is worth. The OTP verdict, the
* super-user resolution, the profile group and the auth token are all computed from IAM's own data.
* Adding a rule here no longer means touching the authentication server.
*
* The response keeps the types the token needs, so the authentication server rebuilds the very same
* principal without depending on the administration model: scalars stay typed, {@code status} and
* {@code type} travel as their enum name, and the five composite attributes ({@code address},
* {@code analytics}, {@code profileGroup}, {@code basicCustomer}, {@code tenantsByApp}) travel as
* their already-serialized JSON, so the bytes reaching the applications are unchanged.
*
* This is not a read-only call, despite the name. Resolving the user mints an authentication token
* and stamps {@code lastConnection}; a subrogation writes its logbook event; and a login through an
* external provider with auto-provisioning enabled creates or updates the account on the way.
*
* @throws NotFoundException when no user matches, which is how a refusal reaches the authentication
* server — never an empty response.
*/
CasController.buildPrincipalAttributes
/**
* The authentication attributes of a user, ready to be carried by the token.
*
* The authentication server no longer derives any of them from the user model: it publishes what
* IAM assembled.
*/
Ce qui change par rapport à l'existant
Retiré, parce que faux :
- « Chaque valeur est une chaîne, booléens et dates compris » et les deux lignes qui l'argumentent (Boolean.parseBoolean, OffsetDateTime.parse) — contredit par le DTO typé ;
- « Un attribut dont la valeur est absente est omis plutôt que mis à null » — invariant inexistant sur un DTO sans @JsonInclude(NON_NULL) ;
- « il recopie la map sans l'interpréter » côté contrôleur.
Conservé, parce que vrai et utile : l'argument sur la fidélité des attributs composites (le JSON pré-sérialisé garantit que les octets reçus par les applications ne bougent pas) — c'est la
justification non évidente du design, elle mérite de rester.
Ajouté : les effets de bord. C'est le manque le plus coûteux du javadoc actuel. Une méthode nommée build… qui écrit un Token en base, met à jour lastConnection, peut créer un utilisateur par
auto-provisionnement et poser un événement de logbook, ça ne se devine pas depuis la signature. C'est aussi ce qui explique pourquoi elle est en POST et pas en GET.
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
3 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.
No description provided.