Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gplazma: multimap fix op regression #7658

Merged
merged 1 commit into from
Sep 10, 2024
Merged

Conversation

paulmillar
Copy link
Member

Motivation:

Commit ef2dc1e introduced a regression in the multimap plugin. Where the 'op' principal type is used, logins will fail with dCache logging a stacktrace like:

java.lang.RuntimeException: Failed to create principal: java.lang.NoSuchMethodException: org.dcache.auth.OAuthProviderPrincipal.<init>(java.lang.String)
        at org.dcache.gplazma.plugins.GplazmaMultiMapFile$MappablePrincipal.buildPrincipal(GplazmaMultiMapFile.java:148)
        at org.dcache.gplazma.plugins.GplazmaMultiMapFile$MappablePrincipal.buildMatcher(GplazmaMultiMapFile.java:163)
        at org.dcache.gplazma.plugins.GplazmaMultiMapFile.asMatcher(GplazmaMultiMapFile.java:270)
        at org.dcache.gplazma.plugins.GplazmaMultiMapFile.parseMapFile(GplazmaMultiMapFile.java:248)
        at org.dcache.gplazma.plugins.GplazmaMultiMapFile.mapping(GplazmaMultiMapFile.java:216)
        at org.dcache.gplazma.plugins.GplazmaMultiMapPlugin.map(GplazmaMultiMapPlugin.java:37)
        at org.dcache.gplazma.strategies.DefaultMappingStrategy.lambda$map$0(DefaultMappingStrategy.java:57)
        at org.dcache.gplazma.strategies.PAMStyleStrategy.callPlugins(PAMStyleStrategy.java:91)

This problem is because the above commit replaced the single-string constructor that was being used by the multimap plugin via reflection.

Modification:

Define the following semantics:

When op is used as a principal matcher, the matcher will be selected when the login has an OAuthProviderPrincipal with that value as its (dCache) name; for example, op:FOO will match if the login has an OAuthProviderPrincipal with name FOO. The URL of the issuer (the iss claim value) is not considered. This recreates the previous semantics.

When used as a principal, the op takes two colon-sparated arguments: the (dCache) name for the OP and the issuer URL (the iss claim value). For example, op:FOO:https://my-op.example.org/ creates an OP with name FOO and issuer URL https://my-op.example.org/.

For backwards compatibility, if the second colon and the issuer URL is omitted then a placeholder URL is used and a warning is logged; for example, op:FOO will add a OAuthProviderPrincipal with name FOO and a placeholder issuer URL.

Unit tests are added that verify correct behaviour.

Result:

A regression is fixed in the multimap plugin when op: principal type is used.

Target: master
Request: 10.1
Request: 10.0
Request: 9.2
Requires-notes: yes
Requires-book: no
Closes: #7654
Patch: https://rb.dcache.org/r/14314/
Acked-by: Tigran Mkrtchyan

Motivation:

Commit ef2dc1e introduced a regression in the multimap plugin.  Where
the 'op' principal type is used, logins will fail with dCache logging
a stacktrace like:

    java.lang.RuntimeException: Failed to create principal: java.lang.NoSuchMethodException: org.dcache.auth.OAuthProviderPrincipal.<init>(java.lang.String)
            at org.dcache.gplazma.plugins.GplazmaMultiMapFile$MappablePrincipal.buildPrincipal(GplazmaMultiMapFile.java:148)
            at org.dcache.gplazma.plugins.GplazmaMultiMapFile$MappablePrincipal.buildMatcher(GplazmaMultiMapFile.java:163)
            at org.dcache.gplazma.plugins.GplazmaMultiMapFile.asMatcher(GplazmaMultiMapFile.java:270)
            at org.dcache.gplazma.plugins.GplazmaMultiMapFile.parseMapFile(GplazmaMultiMapFile.java:248)
            at org.dcache.gplazma.plugins.GplazmaMultiMapFile.mapping(GplazmaMultiMapFile.java:216)
            at org.dcache.gplazma.plugins.GplazmaMultiMapPlugin.map(GplazmaMultiMapPlugin.java:37)
            at org.dcache.gplazma.strategies.DefaultMappingStrategy.lambda$map$0(DefaultMappingStrategy.java:57)
            at org.dcache.gplazma.strategies.PAMStyleStrategy.callPlugins(PAMStyleStrategy.java:91)

This problem is because the above commit replaced the single-string
constructor that was being used by the multimap plugin via reflection.

Modification:

Define the following semantics:

When `op` is used as a principal matcher, the matcher will be selected
when the login has an OAuthProviderPrincipal with that value as its
(dCache) name; for example, `op:FOO` will match if the login has an
OAuthProviderPrincipal with name `FOO`.  The URL of the issuer (the
`iss` claim value) is not considered.  This recreates the previous
semantics.

When used as a principal, the `op` takes two colon-sparated arguments:
the (dCache) name for the OP and the issuer URL (the `iss` claim value).
For example, `op:FOO:https://my-op.example.org/` creates an OP with name
`FOO` and issuer URL `https://my-op.example.org/`.

For backwards compatibility, if the second colon and the issuer URL is
omitted then a placeholder URL is used and a warning is logged; for
example, `op:FOO` will add a OAuthProviderPrincipal with name `FOO` and
a placeholder issuer URL.

Unit tests are added that verify correct behaviour.

Result:

A regression is fixed in the multimap plugin when `op:` principal type
is used.

Target: master
Request: 10.1
Request: 10.0
Request: 9.2
Requires-notes: yes
Requires-book: no
Closes: dCache#7654
Patch: https://rb.dcache.org/r/14314/
Acked-by: Tigran Mkrtchyan
@lemora lemora merged commit 729ce8f into dCache:9.2 Sep 10, 2024
1 check was pending
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants