jerqi commented on issue #9767: URL: https://github.com/apache/gravitino/issues/9767#issuecomment-3787826601
> [@jerqi](https://github.com/jerqi) [@yangyuxia](https://github.com/yangyuxia) Thanks for the feedback! After reviewing the discussion and existing code, I'd like to propose keeping this PR **focused on OAuth only**. Here's my reasoning: > > **For this PR (OAuth user mapping):** > > * OAuth currently has no user mapping - adding pattern with default `^(.*)$` is non-breaking > * Solves the immediate issue (Azure AD email → username extraction) > > **For Kerberos (future PR/issue):** > > * Current behavior: `user/instance@REALM` → returns `user/instance` > * Adding a default pattern would change this behavior (potential breaking change) > * Should have its own config: `gravitino.authenticator.kerberos.userMappingPattern` > * Regex pattern would work alongside KerberosPrincipal (pattern for display name, KerberosPrincipal for delegation tokens) > * Needs proper discussion on defaults and backward compatibility > > **Recommendation:** Keep this PR OAuth-only and file a separate issue for Kerberos user mapping with KerberosPrincipal enhancement. This addresses the immediate OAuth use case while allowing proper consideration for Kerberos requirements. > > Does this approach work? > > Unless you think we should have the same config work for both OAuth and Kerberos? The approach is ok for me. Just two points: 1. The OAuth authenticator and Kerberos authenticator should reuse the same `UserMapping` interface, they can implement their UserMapping class. 2. The map method should return Principal object instead of String object. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
