You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fineract.apache.org by "alberto-art3ch (via GitHub)" <gi...@apache.org> on 2026/05/24 01:54:27 UTC

[PR] FINERACT-2616: OIDC Federation support for IdP [fineract]

alberto-art3ch opened a new pull request, #5883:
URL: https://github.com/apache/fineract/pull/5883

   ## Description
   
   We are adding a complete OIDC (OpenID Connect) Federation layer for Apache Fineract, enabling integration with external Identity Providers (Keycloak, Google, Azure AD, Okta, Auth0) without replacing the existing Basic Auth mechanism.
   
     - OidcFederationSecurityConfig — New Spring Security filter chain (@Order(100)) active only when `fineract.security.oidc-federation.enabled=true`
     - OidcTenantAwareFilter — Resolves the Fineract tenant from a configurable JWT claim (fineract_tenant by default) and sets the multi-tenant context
     - FineractOidcJwtAuthenticationConverter — Maps JWT claims to a FineractOidcUser principal with configurable username claim (preferred_username, email, sub)
     - FineractOidcUserService — Loads existing Fineract AppUser from the resolved OIDC principal; optionally auto-creates users on first login with configurable default roles
     - OidcAuthenticationSuccessHandler / OidcLogoutSuccessHandler — Handles post-login and RP-Initiated Logout per provider dialect (Keycloak, Azure AD, Okta, Auth0, generic)
     - FineractCorsConfiguration — Extracted CORS config as a reusable bean shared across Security filter chains
     - FineractProperties.FineractSecurityOidcFederationProperties — Config block under `fineract.security.oidc-federation`
   
   
   [FINERACT-2616](https://issues.apache.org/jira/browse/FINERACT-2616)
   
   ## Checklist
   
   Please make sure these boxes are checked before submitting your pull request - thanks!
   
   - [ ] Write the commit message as per [our guidelines](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#pull-requests)
   - [ ] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
   - [ ] Create/update [unit or integration tests](https://fineract.apache.org/docs/current/#_testing) for verifying the changes made.
   - [ ] Follow our [coding conventions](https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions).
   - [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
   - [ ] [This PR must not be a "code dump"](https://cwiki.apache.org/confluence/display/FINERACT/Pull+Request+Size+Limit). Large changes can be made in a branch, with assistance. Ask for help on the [developer mailing list](https://fineract.apache.org/#contribute).
   
   Your assigned reviewer(s) will follow our [guidelines for code reviews](https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2616: OIDC Federation support for IdP [fineract]

Posted by "alberto-art3ch (via GitHub)" <gi...@apache.org>.
alberto-art3ch commented on PR #5883:
URL: https://github.com/apache/fineract/pull/5883#issuecomment-4651765255

   > All four review comments from my previous pass have been addressed:
   > 
   > * Spring @transactional is used instead of Jakarta
   > * provider is modeled as OidcFederationType enum
   > * catch block in OidcTenantAwareFilter now logs a warning
   > * The hardcoded 1L for head office is replaced with a configurable fineractProperties.getDefaults().getOfficeId()
   > 
   > The mailing list requirement (per-tenant OIDC configuration, different IdP per tenant) has also been implemented - the m_tenant_oidc_config table, TenantOidcConfigApiResource, and DynamicJwtIssuerAuthenticationManagerResolver address exactly what I asked for on the thread.
   > 
   > Build quality checks still need to be green before merge.
   
   @galovics 
   Quality checks are now in green. Thanks!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2616: OIDC Federation support for IdP [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5883:
URL: https://github.com/apache/fineract/pull/5883#discussion_r3298568955


##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/OidcAppUserResolutionServiceImpl.java:
##########
@@ -0,0 +1,132 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.security.service;
+
+import jakarta.transaction.Transactional;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Stream;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.fineract.infrastructure.core.config.FineractProperties;
+import org.apache.fineract.infrastructure.security.exception.OidcUserNotFoundException;
+import org.apache.fineract.organisation.office.domain.Office;
+import org.apache.fineract.organisation.office.domain.OfficeRepository;
+import org.apache.fineract.useradministration.domain.AppUser;
+import org.apache.fineract.useradministration.domain.AppUserRepository;
+import org.apache.fineract.useradministration.domain.Role;
+import org.apache.fineract.useradministration.domain.RoleRepository;
+import org.springframework.security.core.authority.SimpleGrantedAuthority;
+import org.springframework.security.core.userdetails.User;
+import org.springframework.security.crypto.factory.PasswordEncoderFactories;
+import org.springframework.security.crypto.password.PasswordEncoder;
+import org.springframework.stereotype.Service;
+
+@Slf4j
+@Service
+@RequiredArgsConstructor
+public class OidcAppUserResolutionServiceImpl implements OidcAppUserResolutionService {

Review Comment:
   Raise a conversation about this on dev mail list please....



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2616: OIDC Federation support for IdP [fineract]

Posted by "alberto-art3ch (via GitHub)" <gi...@apache.org>.
alberto-art3ch commented on code in PR #5883:
URL: https://github.com/apache/fineract/pull/5883#discussion_r3300779040


##########
fineract-security/src/main/java/org/apache/fineract/infrastructure/security/handler/OidcAuthenticationSuccessHandler.java:
##########
@@ -0,0 +1,55 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.security.handler;
+
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.fineract.infrastructure.security.data.FineractOidcUser;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.web.authentication.SavedRequestAwareAuthenticationSuccessHandler;
+
+/**
+ * Handles successful browser-based OIDC login (OAuth2 authorization code redirect flow).
+ *
+ * <p>Logs contextual information about the authenticated user and then delegates to
+ * {@link SavedRequestAwareAuthenticationSuccessHandler} so the browser is redirected
+ * back to the originally requested URL.
+ *
+ * <p>This handler is only relevant for the interactive login redirect flow. Bearer token
+ * API requests do not trigger it.
+ */
+@Slf4j
+public class OidcAuthenticationSuccessHandler extends SavedRequestAwareAuthenticationSuccessHandler {
+
+    @Override
+    public void onAuthenticationSuccess(HttpServletRequest request, HttpServletResponse response,
+            Authentication authentication) throws IOException, ServletException {
+
+        if (authentication.getPrincipal() instanceof FineractOidcUser oidcUser) {
+            log.info("OIDC login successful — user: '{}', tenant: '{}', Fineract id: {}",

Review Comment:
   Done!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2616: OIDC Federation support for IdP [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5883:
URL: https://github.com/apache/fineract/pull/5883#discussion_r3298573631


##########
fineract-security/src/main/java/org/apache/fineract/infrastructure/security/filter/OidcTenantAwareFilter.java:
##########
@@ -0,0 +1,113 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.security.filter;
+
+import com.nimbusds.jwt.JWTParser;
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.fineract.infrastructure.core.config.FineractProperties;
+import org.apache.fineract.infrastructure.core.service.ThreadLocalContextUtil;
+import org.apache.fineract.infrastructure.security.service.AuthTenantDetailsService;
+import org.springframework.lang.NonNull;
+import org.springframework.security.oauth2.server.resource.web.BearerTokenResolver;
+import org.springframework.web.filter.OncePerRequestFilter;
+
+/**
+ * Resolves the Fineract tenant from an incoming request and sets it in {@link ThreadLocalContextUtil}
+ * before Spring Security validates the JWT signature.
+ *
+ * <p>Resolution priority:
+ * <ol>
+ *   <li>A configurable claim inside the Bearer JWT (parsed without signature verification)</li>
+ *   <li>The {@code Fineract-Platform-TenantId} HTTP header</li>
+ *   <li>The {@code tenantIdentifier} query parameter</li>
+ * </ol>
+ *
+ * <p>If no tenant can be resolved the filter does not block the request — downstream
+ * authentication will fail with an appropriate error if the tenant context is required.
+ */
+@Slf4j
+@RequiredArgsConstructor
+public class OidcTenantAwareFilter extends OncePerRequestFilter {
+
+    private static final String TENANT_HEADER = "Fineract-Platform-TenantId";
+    private static final String TENANT_PARAM = "tenantIdentifier";
+
+    private final BearerTokenResolver bearerTokenResolver;
+    private final AuthTenantDetailsService tenantDetailsService;
+    private final FineractProperties fineractProperties;
+
+    @Override
+    protected void doFilterInternal(@NonNull HttpServletRequest request, @NonNull HttpServletResponse response,
+            @NonNull FilterChain filterChain) throws ServletException, IOException {
+        try {
+            String tenantId = resolveTenantId(request);
+            if (tenantId != null) {
+                ThreadLocalContextUtil.setTenant(tenantDetailsService.loadTenantById(tenantId, false));
+                log.debug("OIDC tenant context set to '{}' for {}", tenantId, request.getRequestURI());
+            }
+            filterChain.doFilter(request, response);
+        } catch (Exception e) {
+            // don't block; real auth will fail later if token or tenant is invalid
+            filterChain.doFilter(request, response);

Review Comment:
   I dont think it is wise to not fail ASAP if no tenant information is provided...



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2616: OIDC Federation support for IdP [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5883:
URL: https://github.com/apache/fineract/pull/5883#discussion_r3298576535


##########
fineract-security/src/main/java/org/apache/fineract/infrastructure/security/handler/OidcAuthenticationSuccessHandler.java:
##########
@@ -0,0 +1,55 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.security.handler;
+
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.fineract.infrastructure.security.data.FineractOidcUser;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.web.authentication.SavedRequestAwareAuthenticationSuccessHandler;
+
+/**
+ * Handles successful browser-based OIDC login (OAuth2 authorization code redirect flow).
+ *
+ * <p>Logs contextual information about the authenticated user and then delegates to
+ * {@link SavedRequestAwareAuthenticationSuccessHandler} so the browser is redirected
+ * back to the originally requested URL.
+ *
+ * <p>This handler is only relevant for the interactive login redirect flow. Bearer token
+ * API requests do not trigger it.
+ */
+@Slf4j
+public class OidcAuthenticationSuccessHandler extends SavedRequestAwareAuthenticationSuccessHandler {
+
+    @Override
+    public void onAuthenticationSuccess(HttpServletRequest request, HttpServletResponse response,
+            Authentication authentication) throws IOException, ServletException {
+
+        if (authentication.getPrincipal() instanceof FineractOidcUser oidcUser) {
+            log.info("OIDC login successful — user: '{}', tenant: '{}', Fineract id: {}",

Review Comment:
   Should be debug level



##########
fineract-security/src/main/java/org/apache/fineract/infrastructure/security/handler/OidcLogoutSuccessHandler.java:
##########
@@ -0,0 +1,123 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.security.handler;
+
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.fineract.infrastructure.core.config.FineractProperties;
+import org.apache.fineract.infrastructure.security.data.FineractOidcUser;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.web.authentication.logout.LogoutSuccessHandler;
+import org.springframework.security.web.authentication.logout.SimpleUrlLogoutSuccessHandler;
+import org.springframework.web.util.UriComponentsBuilder;
+
+@Slf4j
+@RequiredArgsConstructor
+public class OidcLogoutSuccessHandler implements LogoutSuccessHandler {
+
+    private static final String FALLBACK_URL = "/login?logout";
+
+    private final FineractProperties fineractProperties;
+
+    @Override
+    public void onLogoutSuccess(HttpServletRequest request, HttpServletResponse response,
+            Authentication authentication) throws IOException, ServletException {
+
+        if (authentication != null && authentication.getPrincipal() instanceof FineractOidcUser oidcUser) {
+            log.info("OIDC logout initiated for user '{}' (tenant: '{}')",

Review Comment:
   Should be debug level



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2616: OIDC Federation support for IdP [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5883:
URL: https://github.com/apache/fineract/pull/5883#discussion_r3298558160


##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/config/FineractCorsConfiguration.java:
##########
@@ -0,0 +1,50 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.core.config;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.web.cors.CorsConfiguration;
+import org.springframework.web.cors.CorsConfigurationSource;
+import org.springframework.web.cors.UrlBasedCorsConfigurationSource;
+
+@Configuration
+public class FineractCorsConfiguration {
+
+    @Autowired
+    private FineractProperties fineractProperties;
+
+    @Bean
+    @ConditionalOnMissingBean(CorsConfigurationSource.class)
+    public CorsConfigurationSource corsConfigurationSource() {

Review Comment:
   Already exists:
   `org.apache.fineract.infrastructure.core.config.SecurityConfig#corsConfigurationSource`



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/config/FineractCorsConfiguration.java:
##########
@@ -0,0 +1,50 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.core.config;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.web.cors.CorsConfiguration;
+import org.springframework.web.cors.CorsConfigurationSource;
+import org.springframework.web.cors.UrlBasedCorsConfigurationSource;
+
+@Configuration
+public class FineractCorsConfiguration {
+
+    @Autowired
+    private FineractProperties fineractProperties;
+
+    @Bean
+    @ConditionalOnMissingBean(CorsConfigurationSource.class)
+    public CorsConfigurationSource corsConfigurationSource() {

Review Comment:
   Already exists:
   `org.apache.fineract.infrastructure.core.config.SecurityConfig#corsConfigurationSource`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2616: OIDC Federation support for IdP [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #5883:
URL: https://github.com/apache/fineract/pull/5883


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2616: OIDC Federation support for IdP [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5883:
URL: https://github.com/apache/fineract/pull/5883#discussion_r3298363656


##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/core/config/FineractProperties.java:
##########
@@ -556,6 +557,27 @@ public static final class Registration implements Serializable {
             }
         }
 
+        @Getter
+        @Setter
+        public static class FineractSecurityOidcFederationProperties {

Review Comment:
   This looks messy to me...
   Why should we have any of the below fields as configuration:
   - tenantClaimName
   - usernameClaim
   - autoCreateUser
   - defaultRoles



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2616: OIDC Federation support for IdP [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5883:
URL: https://github.com/apache/fineract/pull/5883#discussion_r3298575386


##########
fineract-security/src/main/java/org/apache/fineract/infrastructure/security/filter/OidcTenantAwareFilter.java:
##########
@@ -0,0 +1,113 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.security.filter;
+
+import com.nimbusds.jwt.JWTParser;
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.fineract.infrastructure.core.config.FineractProperties;
+import org.apache.fineract.infrastructure.core.service.ThreadLocalContextUtil;
+import org.apache.fineract.infrastructure.security.service.AuthTenantDetailsService;
+import org.springframework.lang.NonNull;
+import org.springframework.security.oauth2.server.resource.web.BearerTokenResolver;
+import org.springframework.web.filter.OncePerRequestFilter;
+
+/**
+ * Resolves the Fineract tenant from an incoming request and sets it in {@link ThreadLocalContextUtil}
+ * before Spring Security validates the JWT signature.
+ *
+ * <p>Resolution priority:
+ * <ol>
+ *   <li>A configurable claim inside the Bearer JWT (parsed without signature verification)</li>
+ *   <li>The {@code Fineract-Platform-TenantId} HTTP header</li>
+ *   <li>The {@code tenantIdentifier} query parameter</li>
+ * </ol>
+ *
+ * <p>If no tenant can be resolved the filter does not block the request — downstream
+ * authentication will fail with an appropriate error if the tenant context is required.
+ */
+@Slf4j
+@RequiredArgsConstructor
+public class OidcTenantAwareFilter extends OncePerRequestFilter {
+
+    private static final String TENANT_HEADER = "Fineract-Platform-TenantId";
+    private static final String TENANT_PARAM = "tenantIdentifier";
+
+    private final BearerTokenResolver bearerTokenResolver;
+    private final AuthTenantDetailsService tenantDetailsService;
+    private final FineractProperties fineractProperties;
+
+    @Override
+    protected void doFilterInternal(@NonNull HttpServletRequest request, @NonNull HttpServletResponse response,
+            @NonNull FilterChain filterChain) throws ServletException, IOException {
+        try {
+            String tenantId = resolveTenantId(request);
+            if (tenantId != null) {
+                ThreadLocalContextUtil.setTenant(tenantDetailsService.loadTenantById(tenantId, false));
+                log.debug("OIDC tenant context set to '{}' for {}", tenantId, request.getRequestURI());
+            }
+            filterChain.doFilter(request, response);
+        } catch (Exception e) {
+            // don't block; real auth will fail later if token or tenant is invalid
+            filterChain.doFilter(request, response);
+        } finally {
+            ThreadLocalContextUtil.reset();
+        }
+    }
+
+    private String resolveTenantId(HttpServletRequest request) {
+        // 1. JWT claim (no signature verification — same strategy as TenantAwareAuthenticationFilter)
+        String claimName = fineractProperties.getSecurity().getOidcFederation().getTenantClaimName();
+        if (claimName != null) {
+            try {
+                String token = bearerTokenResolver.resolve(request);
+                if (token != null) {
+                    var claims = JWTParser.parse(token).getJWTClaimsSet();
+                    String fromClaim = (String) claims.getClaim(claimName);
+                    if (fromClaim != null) {
+                        return fromClaim;
+                    }
+                }
+            } catch (Exception ignored) {
+                // malformed token — fall through to header / query param

Review Comment:
   I dont like this approach to ignore malformed token...



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2616: OIDC Federation support for IdP [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5883:
URL: https://github.com/apache/fineract/pull/5883#discussion_r3298363656


##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/core/config/FineractProperties.java:
##########
@@ -556,6 +557,27 @@ public static final class Registration implements Serializable {
             }
         }
 
+        @Getter
+        @Setter
+        public static class FineractSecurityOidcFederationProperties {

Review Comment:
   This looks messy to me...
   Why should we have any of the below fields as configuration:
   - tenantClaimName -> There should be no default value
   - usernameClaim -> There should be no default value
   - autoCreateUser -> There should be no such functionality as part of OIDC, maybe as a follow-up...
   - defaultRoles? -> No default roles... security nightmare AS-IS



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2616: OIDC Federation support for IdP [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #5883:
URL: https://github.com/apache/fineract/pull/5883#issuecomment-4534552359

   @alberto-art3ch Please review the failing quality checks.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2616: OIDC Federation support for IdP [fineract]

Posted by "alberto-art3ch (via GitHub)" <gi...@apache.org>.
alberto-art3ch commented on code in PR #5883:
URL: https://github.com/apache/fineract/pull/5883#discussion_r3300834353


##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/OidcAppUserResolutionServiceImpl.java:
##########
@@ -0,0 +1,132 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.security.service;
+
+import jakarta.transaction.Transactional;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Stream;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.fineract.infrastructure.core.config.FineractProperties;
+import org.apache.fineract.infrastructure.security.exception.OidcUserNotFoundException;
+import org.apache.fineract.organisation.office.domain.Office;
+import org.apache.fineract.organisation.office.domain.OfficeRepository;
+import org.apache.fineract.useradministration.domain.AppUser;
+import org.apache.fineract.useradministration.domain.AppUserRepository;
+import org.apache.fineract.useradministration.domain.Role;
+import org.apache.fineract.useradministration.domain.RoleRepository;
+import org.springframework.security.core.authority.SimpleGrantedAuthority;
+import org.springframework.security.core.userdetails.User;
+import org.springframework.security.crypto.factory.PasswordEncoderFactories;
+import org.springframework.security.crypto.password.PasswordEncoder;
+import org.springframework.stereotype.Service;
+
+@Slf4j
+@Service
+@RequiredArgsConstructor
+public class OidcAppUserResolutionServiceImpl implements OidcAppUserResolutionService {

Review Comment:
   Email sent!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2616: OIDC Federation support for IdP [fineract]

Posted by "galovics (via GitHub)" <gi...@apache.org>.
galovics commented on code in PR #5883:
URL: https://github.com/apache/fineract/pull/5883#discussion_r3302675714


##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/core/config/FineractProperties.java:
##########
@@ -556,6 +557,27 @@ public static final class Registration implements Serializable {
             }
         }
 
+        @Getter
+        @Setter
+        public static class FineractSecurityOidcFederationProperties {
+
+            private boolean enabled;
+            // JWT claim name used to resolve the Fineract tenant ID.
+            // Falls back to HTTP header / query param if absent.
+            private String tenantClaimName = "fineract_tenant";
+            // Claim used as the Fineract username. Common values: preferred_username, email, sub.
+            private String usernameClaim = "preferred_username";
+            // When true, creates a Fineract AppUser on first successful OIDC login.
+            private boolean autoCreateUser = false;
+            // Comma-separated role names assigned to auto-created users.
+            private String defaultRoles = "";
+            // Controls the RP-Initiated Logout URL format.
+            // Values: keycloak | azure_ad | okta | auth0 | generic (default)
+            private String provider = "generic";

Review Comment:
   Wouldn't it make more sense to model `provider` as an enum? The `switch` in `OidcLogoutSuccessHandler.buildLogoutUrl()` operates on these exact string values, so a misconfiguration (`azure-ad` instead of `azure_ad`, say) silently falls through to `default -> null` and the logout just never reaches the IdP - no error, no warning. An enum bound via Spring's relaxed binding (`OidcProvider { KEYCLOAK, AZURE_AD, OKTA, AUTH0, GENERIC }`) would catch that at startup. Thoughts?



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/OidcAppUserResolutionServiceImpl.java:
##########
@@ -0,0 +1,125 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.security.service;
+
+import jakarta.transaction.Transactional;

Review Comment:
   Please use `org.springframework.transaction.annotation.Transactional` here instead of the Jakarta one. The Jakarta `@Transactional` has subtly different propagation defaults in a Spring-managed context and can behave unexpectedly — for example around `REQUIRED` propagation when an outer Spring transaction is already active. This is something we try to keep consistent across the codebase.



##########
fineract-security/src/main/java/org/apache/fineract/infrastructure/security/filter/OidcTenantAwareFilter.java:
##########
@@ -0,0 +1,114 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.security.filter;
+
+import com.nimbusds.jwt.JWTParser;
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.fineract.infrastructure.core.config.FineractProperties;
+import org.apache.fineract.infrastructure.core.service.ThreadLocalContextUtil;
+import org.apache.fineract.infrastructure.security.service.AuthTenantDetailsService;
+import org.springframework.lang.NonNull;
+import org.springframework.security.oauth2.server.resource.web.BearerTokenResolver;
+import org.springframework.web.filter.OncePerRequestFilter;
+
+/**
+ * Resolves the Fineract tenant from an incoming request and sets it in {@link ThreadLocalContextUtil} before Spring
+ * Security validates the JWT signature.
+ *
+ * <p>
+ * Resolution priority:
+ * <ol>
+ * <li>A configurable claim inside the Bearer JWT (parsed without signature verification)</li>
+ * <li>The {@code Fineract-Platform-TenantId} HTTP header</li>
+ * <li>The {@code tenantIdentifier} query parameter</li>
+ * </ol>
+ *
+ * <p>
+ * If no tenant can be resolved the filter does not block the request — downstream authentication will fail with an
+ * appropriate error if the tenant context is required.
+ */
+@Slf4j
+@RequiredArgsConstructor
+public class OidcTenantAwareFilter extends OncePerRequestFilter {
+
+    private static final String TENANT_HEADER = "Fineract-Platform-TenantId";
+    private static final String TENANT_PARAM = "tenantIdentifier";
+
+    private final BearerTokenResolver bearerTokenResolver;
+    private final AuthTenantDetailsService tenantDetailsService;
+    private final FineractProperties fineractProperties;
+
+    @Override
+    protected void doFilterInternal(@NonNull HttpServletRequest request, @NonNull HttpServletResponse response,
+            @NonNull FilterChain filterChain) throws ServletException, IOException {
+        try {
+            String tenantId = resolveTenantId(request);
+            if (tenantId != null) {
+                ThreadLocalContextUtil.setTenant(tenantDetailsService.loadTenantById(tenantId, false));
+                log.debug("OIDC tenant context set to '{}' for {}", tenantId, request.getRequestURI());
+            }
+            filterChain.doFilter(request, response);
+        } catch (Exception e) {
+            // don't block; real auth will fail later if token or tenant is invalid

Review Comment:
   I understand the reasoning - don't block the chain - but with no log line at all, nobody will know when this catch triggers. If the tenant service is down, for example, every OIDC request silently skips tenant resolution and you'd have to infer it from downstream errors. Could we at least add `log.debug("Tenant resolution failed, continuing without tenant context", e)` here? Wdyt?



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/service/OidcAppUserResolutionServiceImpl.java:
##########
@@ -0,0 +1,125 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.security.service;
+
+import jakarta.transaction.Transactional;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Stream;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.fineract.infrastructure.core.config.FineractProperties;
+import org.apache.fineract.infrastructure.security.exception.OidcUserNotFoundException;
+import org.apache.fineract.organisation.office.domain.Office;
+import org.apache.fineract.organisation.office.domain.OfficeRepository;
+import org.apache.fineract.useradministration.domain.AppUser;
+import org.apache.fineract.useradministration.domain.AppUserRepository;
+import org.apache.fineract.useradministration.domain.Role;
+import org.apache.fineract.useradministration.domain.RoleRepository;
+import org.springframework.security.core.authority.SimpleGrantedAuthority;
+import org.springframework.security.core.userdetails.User;
+import org.springframework.security.crypto.factory.PasswordEncoderFactories;
+import org.springframework.security.crypto.password.PasswordEncoder;
+import org.springframework.stereotype.Service;
+
+@Slf4j
+@Service
+@RequiredArgsConstructor
+public class OidcAppUserResolutionServiceImpl implements OidcAppUserResolutionService {
+
+    private final AppUserRepository appUserRepository;
+    private final RoleRepository roleRepository;
+    private final OfficeRepository officeRepository;
+    private final FineractProperties fineractProperties;
+
+    // Stateless encoder — safe to create once per class
+    private static final PasswordEncoder PASSWORD_ENCODER = PasswordEncoderFactories.createDelegatingPasswordEncoder();
+
+    @Override
+    @Transactional
+    public AppUser resolveOrCreate(String username, String email, String firstName, String lastName, Set<String> requestedRoles) {
+
+        // 1. Lookup by username
+        AppUser user = appUserRepository.findAppUserByName(username);
+        if (user != null) {
+            log.debug("OIDC user resolved by username: '{}'", username);
+            return user;
+        }
+
+        // 2. Fallback: lookup by email
+        if (email != null) {
+            user = appUserRepository.findActiveUserByEmail(email);
+            if (user != null) {
+                log.debug("OIDC user resolved by email: '{}'", email);
+                return user;
+            }
+        }
+
+        // 3. Auto-create when enabled
+        FineractProperties.FineractSecurityProperties.FineractSecurityOidcFederationProperties oidcConfig = fineractProperties.getSecurity()
+                .getOidcFederation();
+
+        if (!oidcConfig.isAutoCreateUser()) {
+            log.warn("OIDC user '{}' not found in Fineract and auto-create is disabled", username);
+            throw new OidcUserNotFoundException(username);
+        }
+
+        log.info("Auto-creating Fineract user for OIDC subject '{}'", username);
+        return createUser(username, email, firstName, lastName, requestedRoles, oidcConfig);
+    }
+
+    private AppUser createUser(String username, String email, String firstName, String lastName, Set<String> requestedRoles,
+            FineractProperties.FineractSecurityProperties.FineractSecurityOidcFederationProperties oidcConfig) {
+

Review Comment:
   Can we extract `1L` to a named constant - something like `HEAD_OFFICE_ID`? It's a Fineract convention that the head office always has id=1, but a bare `1L` reads like a magic number. Thanks.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2616: OIDC Federation support for IdP [fineract]

Posted by "alberto-art3ch (via GitHub)" <gi...@apache.org>.
alberto-art3ch commented on code in PR #5883:
URL: https://github.com/apache/fineract/pull/5883#discussion_r3300777759


##########
fineract-security/src/main/java/org/apache/fineract/infrastructure/security/handler/OidcLogoutSuccessHandler.java:
##########
@@ -0,0 +1,123 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.security.handler;
+
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.fineract.infrastructure.core.config.FineractProperties;
+import org.apache.fineract.infrastructure.security.data.FineractOidcUser;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.web.authentication.logout.LogoutSuccessHandler;
+import org.springframework.security.web.authentication.logout.SimpleUrlLogoutSuccessHandler;
+import org.springframework.web.util.UriComponentsBuilder;
+
+@Slf4j
+@RequiredArgsConstructor
+public class OidcLogoutSuccessHandler implements LogoutSuccessHandler {
+
+    private static final String FALLBACK_URL = "/login?logout";
+
+    private final FineractProperties fineractProperties;
+
+    @Override
+    public void onLogoutSuccess(HttpServletRequest request, HttpServletResponse response,
+            Authentication authentication) throws IOException, ServletException {
+
+        if (authentication != null && authentication.getPrincipal() instanceof FineractOidcUser oidcUser) {
+            log.info("OIDC logout initiated for user '{}' (tenant: '{}')",

Review Comment:
   Done!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2616: OIDC Federation support for IdP [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5883:
URL: https://github.com/apache/fineract/pull/5883#discussion_r3298566660


##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/config/OidcFederationSecurityConfig.java:
##########
@@ -0,0 +1,245 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.infrastructure.security.config;
+
+import org.apache.fineract.infrastructure.core.config.FineractProperties;
+import org.apache.fineract.infrastructure.core.domain.FineractRequestContextHolder;
+import org.apache.fineract.infrastructure.core.filters.CallerIpTrackingFilter;
+import org.apache.fineract.infrastructure.core.filters.CorrelationHeaderFilter;
+import org.apache.fineract.infrastructure.core.filters.IdempotencyStoreFilter;
+import org.apache.fineract.infrastructure.core.filters.IdempotencyStoreHelper;
+import org.apache.fineract.infrastructure.core.filters.RequestResponseFilter;
+import org.apache.fineract.infrastructure.core.service.MDCWrapper;
+import org.apache.fineract.infrastructure.instancemode.filter.FineractInstanceModeApiFilter;
+import org.apache.fineract.infrastructure.jobs.filter.LoanCOBApiFilter;
+import org.apache.fineract.infrastructure.jobs.filter.LoanCOBFilterHelper;
+import org.apache.fineract.infrastructure.jobs.filter.ProgressiveLoanModelCheckerFilter;
+import org.apache.fineract.infrastructure.security.converter.FineractOidcJwtAuthenticationConverter;
+import org.apache.fineract.infrastructure.security.filter.BusinessDateFilter;
+import org.apache.fineract.infrastructure.security.filter.OidcTenantAwareFilter;
+import org.apache.fineract.infrastructure.security.handler.OidcAuthenticationSuccessHandler;
+import org.apache.fineract.infrastructure.security.handler.OidcLogoutSuccessHandler;
+import org.apache.fineract.infrastructure.security.service.AuthTenantDetailsService;
+import org.apache.fineract.infrastructure.security.service.FineractOidcUserService;
+import org.apache.fineract.infrastructure.businessdate.service.BusinessDateReadPlatformService;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
+import org.springframework.boot.context.properties.EnableConfigurationProperties;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.core.annotation.Order;
+import org.springframework.http.HttpMethod;
+import org.springframework.security.config.Customizer;
+import org.springframework.security.config.annotation.web.builders.HttpSecurity;
+import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
+import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer;
+import org.springframework.security.config.http.SessionCreationPolicy;
+import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository;
+import org.springframework.security.oauth2.server.resource.web.DefaultBearerTokenResolver;
+import org.springframework.security.web.SecurityFilterChain;
+import org.springframework.security.web.access.ExceptionTranslationFilter;
+import org.springframework.security.web.context.SecurityContextHolderFilter;
+
+/**
+ * Spring Security filter chain for OIDC Federation authentication.
+ *
+ * <p>Active only when {@code fineract.security.oidc-federation.enabled=true}. Sits at
+ * {@code @Order(100)} — after the Authorization Server chains ({@code @Order} 1–3) and before
+ * the Basic Auth chain (no {@code @Order}, defaults to lowest priority).
+ *
+ * <p>Provides two authentication modes for {@code /api/**}:
+ * <ul>
+ *   <li><b>Bearer JWT</b> — external IdP access tokens validated via the configured JWK set URI.</li>
+ *   <li><b>OAuth2 login redirect</b> — browser-based flow (requires
+ *       {@code spring.security.oauth2.client.registration.*} properties).</li>
+ * </ul>
+ *
+ * <p>{@code @EnableMethodSecurity} is intentionally omitted. When Basic Auth is also enabled
+ * ({@code SecurityConfig} is loaded), it already registers method security. When only OIDC
+ * is enabled, add {@code @EnableMethodSecurity} to a separate configuration class.
+ */
+@Configuration
+@EnableWebSecurity
+@ConditionalOnProperty("fineract.security.oidc-federation.enabled")
+@Order(100)
+@EnableConfigurationProperties(FineractProperties.class)
+public class OidcFederationSecurityConfig {
+
+    @Autowired
+    private FineractProperties fineractProperties;
+
+    @Autowired
+    private FineractOidcUserService oidcUserService;
+
+    @Autowired
+    private AuthTenantDetailsService tenantDetailsService;
+
+    @Autowired
+    private BusinessDateReadPlatformService businessDateReadPlatformService;
+
+    @Autowired
+    private MDCWrapper mdcWrapper;
+
+    @Autowired
+    private FineractRequestContextHolder fineractRequestContextHolder;
+
+    @Autowired
+    private IdempotencyStoreHelper idempotencyStoreHelper;
+
+    @Autowired
+    private ProgressiveLoanModelCheckerFilter progressiveLoanModelCheckerFilter;
+
+    @Autowired(required = false)
+    private LoanCOBFilterHelper loanCOBFilterHelper;
+
+    // Optional: only needed for browser-based OAuth2 login redirect flow.
+    // Not required for Bearer token API authentication.
+    @Autowired(required = false)
+    private ClientRegistrationRepository clientRegistrationRepository;
+
+    @Bean
+    @Order(100)
+    public SecurityFilterChain oidcFederationFilterChain(HttpSecurity http) throws Exception {
+        http.securityMatcher("/api/**")
+                .authorizeHttpRequests(auth -> auth
+                        .requestMatchers(HttpMethod.OPTIONS, "/api/**").permitAll()
+                        .requestMatchers(HttpMethod.POST, "/api/*/echo").permitAll()
+                        .anyRequest().authenticated())
+                .csrf(AbstractHttpConfigurer::disable)
+                .sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.STATELESS))
+                // Bearer JWT validation — external IdP tokens
+                .oauth2ResourceServer(resourceServer -> resourceServer
+                        .jwt(jwt -> jwt.jwtAuthenticationConverter(oidcJwtConverter())))
+                // Tenant resolution before Spring validates the JWT
+                .addFilterBefore(oidcTenantAwareFilter(), SecurityContextHolderFilter.class)
+                .addFilterAfter(businessDateFilter(), OidcTenantAwareFilter.class)
+                .addFilterAfter(requestResponseFilter(), ExceptionTranslationFilter.class)
+                .addFilterAfter(correlationHeaderFilter(), RequestResponseFilter.class)
+                .addFilterAfter(fineractInstanceModeApiFilter(), CorrelationHeaderFilter.class);
+
+        // LoanCOB and idempotency filters (same ordering as SecurityConfig and AuthorizationServerConfig)
+        if (loanCOBFilterHelper != null) {
+            http.addFilterAfter(loanCOBApiFilter(), FineractInstanceModeApiFilter.class)
+                    .addFilterAfter(idempotencyStoreFilter(), LoanCOBApiFilter.class)
+                    .addFilterBefore(progressiveLoanModelCheckerFilter, LoanCOBApiFilter.class);
+        } else {
+            http.addFilterAfter(idempotencyStoreFilter(), FineractInstanceModeApiFilter.class)
+                    .addFilterAfter(progressiveLoanModelCheckerFilter, FineractInstanceModeApiFilter.class);
+        }
+
+        if (fineractProperties.getIpTracking().isEnabled()) {
+            http.addFilterAfter(callerIpTrackingFilter(), RequestResponseFilter.class);
+        }
+
+        // Optional: browser-based OAuth2 login redirect flow
+        if (clientRegistrationRepository != null) {
+            http.oauth2Login(oauth2 -> oauth2
+                    .clientRegistrationRepository(clientRegistrationRepository)
+                    .userInfoEndpoint(userInfo -> userInfo
+                            .oidcUserService(oidcSpringUserService()))
+                    .successHandler(new OidcAuthenticationSuccessHandler())
+                    .failureUrl("/login?error=true"))
+                    .logout(logout -> logout
+                            .logoutUrl("/logout")
+                            .logoutSuccessHandler(oidcLogoutSuccessHandler())
+                            .invalidateHttpSession(true)
+                            .clearAuthentication(true)
+                            .deleteCookies("JSESSIONID"));
+        }
+
+        if (fineractProperties.getSecurity().getCors().isEnabled()) {
+            http.cors(Customizer.withDefaults());
+        }
+
+        return http.build();
+    }
+
+    // -------------------------------------------------------------------------
+    // Managed beans — kept as @Bean for testability
+    // -------------------------------------------------------------------------
+
+    @Bean
+    public FineractOidcJwtAuthenticationConverter oidcJwtConverter() {
+        return new FineractOidcJwtAuthenticationConverter(oidcUserService);
+    }
+
+    @Bean
+    public OidcTenantAwareFilter oidcTenantAwareFilter() {
+        return new OidcTenantAwareFilter(new DefaultBearerTokenResolver(), tenantDetailsService,
+                fineractProperties);
+    }
+
+    // -------------------------------------------------------------------------
+    // Plain factory methods — NOT @Bean — to avoid duplicate bean registration
+    // when SecurityConfig or AuthorizationServerConfig is also active
+    // -------------------------------------------------------------------------
+
+    public BusinessDateFilter businessDateFilter() {
+        return new BusinessDateFilter(businessDateReadPlatformService);
+    }
+
+    public RequestResponseFilter requestResponseFilter() {
+        return new RequestResponseFilter();
+    }
+
+    public CorrelationHeaderFilter correlationHeaderFilter() {
+        return new CorrelationHeaderFilter(fineractProperties, mdcWrapper);
+    }
+
+    public FineractInstanceModeApiFilter fineractInstanceModeApiFilter() {
+        return new FineractInstanceModeApiFilter(fineractProperties);
+    }
+
+    public LoanCOBApiFilter loanCOBApiFilter() {
+        return new LoanCOBApiFilter(loanCOBFilterHelper);
+    }
+
+    public IdempotencyStoreFilter idempotencyStoreFilter() {
+        return new IdempotencyStoreFilter(fineractRequestContextHolder, idempotencyStoreHelper,
+                fineractProperties);
+    }
+
+    public CallerIpTrackingFilter callerIpTrackingFilter() {
+        return new CallerIpTrackingFilter(fineractProperties);
+    }
+
+    public OidcLogoutSuccessHandler oidcLogoutSuccessHandler() {
+        return new OidcLogoutSuccessHandler(fineractProperties);
+    }
+
+    /**
+     * Wraps Spring's OidcUserService to process the OIDC user through Fineract's resolution
+     * logic during the browser login redirect flow.
+     */
+    private org.springframework.security.oauth2.client.userinfo.OAuth2UserService<
+            org.springframework.security.oauth2.client.oidc.userinfo.OidcUserRequest,
+            org.springframework.security.oauth2.core.oidc.user.OidcUser> oidcSpringUserService() {
+
+        var delegate = new org.springframework.security.oauth2.client.oidc.userinfo.OidcUserService();
+        return userRequest -> {
+            var oidcUser = delegate.loadUser(userRequest);
+            // Tenant ID is not available here; OidcTenantAwareFilter sets it from the token
+            String tenantId = fineractProperties.getSecurity().getOidcFederation().getTenantClaimName();
+            String resolvedTenant = oidcUser.getClaimAsString(tenantId) != null

Review Comment:
   I dont like this fallback logic... i dont think there should be such



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-2616: OIDC Federation support for IdP [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #5883:
URL: https://github.com/apache/fineract/pull/5883#issuecomment-4659944208

   @IOhacker Kindly asking you to update your review


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org