[
https://issues.apache.org/jira/browse/SLING-12714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17958774#comment-17958774
]
Robert Munteanu commented on SLING-12714:
-----------------------------------------
Thanks for the PR [~nscendoni] - only one minor comment. Looking at the tests
this lead me down a rabbit hole (and fully my fault for not reviewing the
initial PR in a timely manner )
- I saw in
https://github.com/apache/sling-org-apache-sling-auth-oauth-client/blob/449b3feeadd93e1d667081ed9ebeb58951c78d7c/src/test/java/org/apache/sling/auth/oauth_client/impl/OidcAuthenticationHandlerTest.java#L95
that you are mocking the SlingRepository . That to me is a smell because we
already have proper mocks for the repository
- All the OidcAuthenticationHandler does with the SlingRepository is pass it to
the LoginCookieManager as a parameter -
https://github.com/apache/sling-org-apache-sling-auth-oauth-client/blob/449b3feeadd93e1d667081ed9ebeb58951c78d7c/src/main/java/org/apache/sling/auth/oauth_client/impl/OidcAuthenticationHandler.java#L551
Why do we need to have a reference to the {{SlingRepository}} in the API? The
default SlingLoginCookieManager does not even use this parameter -
https://github.com/apache/sling-org-apache-sling-auth-oauth-client/blob/449b3feeadd93e1d667081ed9ebeb58951c78d7c/src/main/java/org/apache/sling/auth/oauth_client/impl/SlingLoginCookieManager.java#L89-L107
.
I think it would be preferrable for custom implementations of the
{{LoginCookieManager}} to make this a requirement via declarative service (
{{@Reference}} ) and remove this from the API.
In that direction, since we ship a default {{SlingLoginCookieManager}}, do we
need to make the reference optional? See
https://github.com/apache/sling-org-apache-sling-auth-oauth-client/blob/449b3feeadd93e1d667081ed9ebeb58951c78d7c/src/main/java/org/apache/sling/auth/oauth_client/impl/OidcAuthenticationHandler.java#L155-L156
> Oidc Authentication Handler
> ---------------------------
>
> Key: SLING-12714
> URL: https://issues.apache.org/jira/browse/SLING-12714
> Project: Sling
> Issue Type: Improvement
> Components: Extensions
> Reporter: Nicola Scendoni
> Assignee: Nicola Scendoni
> Priority: Minor
> Fix For: OAuth Client 0.1.2
>
>
> {*}Description:{*}{*}{*}
> Apache Sling currently provides an *OAuth 2 client*, but it is *not an
> authentication handler*. This means that while applications can use OAuth 2
> for authorization, there is no built-in mechanism to handle user
> authentication via OpenID Connect (OIDC). Given the widespread adoption of
> OIDC for authentication, adding support for an *OIDC Authentication Handler*
> would greatly enhance Sling’s authentication capabilities.
>
> {*}Feature Request:{*}{*}{*}
> Develop a pluggable *OIDC Authentication Handler* that enables authentication
> via OpenID Connect providers (e.g., Google, Azure AD, Keycloak, Okta).
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
