Add OpenID Response Type

[marcriemer]

No description provided.

[doobry-systemli]

I'm glad to see some progress here! @marcriemer do you intend to continue working on it? Did you get stuck somewhere particularly?

[marcriemer]

@doobry-systemli So far my implementation works very well. I will continue my work on in this feature. Please let me know If you have any successions for improvements.

[Sephster]

Version 9 bringing in the device code is being worked on just now. Then I will start to look at other features. V9 RC1 should be tagged today all being well

[marcriemer]

@pat0s Thanks for bringing this up. You are right, the method should return a ClaimSetEntryInterface.

The ClaimSetEntryInterface extends ClaimSetInterface.

[SimLibaud]

Hi @marcriemer,

Thanks for the initiative and your work. It's a very interesting feature.

@Sephster Have you any news about this PR ?

[stof]

getClaims is documented as returning string[] while the extractor expects array<string, string>. This looks inconsistent to me.

[stof]

To me, this looks like we need 2 separate interface: one returning the map of claim names to claim values (used by the ClaimRepository when getting the claims for an access token) and one listing a set of claim names associated with a scope.

Even if both interfaces have a method named getClaims, the would still be separate interface due to different signatures.

[stof]

btw, rebasing this PR would probably make the CI red because phpstan should be complaining about that.

[marcriemer]

Well, the ClaimExtractor needs to be fixed. It extracts ClaimSetEntryInterface, therefore most vars and properties are completely off.

[marcriemer]

Implementation of a ClaimSetInterface used by the ClaimExtractor could look like this:

    /**
     * Get users claims
     * 
     * @return array<string, string>
     */
    public function getClaims(): array
    {
        return [
            'preferred_username' => $this->getUsername(),
            'email' => $this->getEmail(),
        ];
    }

[stof]

but ClaimSetEntryInterface currently extends ClaimSet. this is what looks wrong to me as it seems like a separate concern (on one side, we associate a scope with a set of claim names, and on the other side, we have a map of claim names to claim values, which is not a set at all but a map)

[marcriemer]

The ClaimExtractor was confusing: marcriemer@b1400a1

[stof]

@marcriemer @Sephster what is the status of this work ?

[stof]

shouldn't the nonce come from the authorization request ? Configuring as a constructor argument of the repository is probably not usable. It would be great to have an actual example of the openid setup.

[marcriemer]

Agree, the example is far from being perfect. The nonce must be part of the AuthCodeEntityInterface and used by ClaimSetRepositoryInterface to create ClaimSetInterface with the nonce claim.

[marcriemer]

@stof I would love to contribute more, but some commits are sitting on the sideline right now. My complete OIDC implementation is nearly finished, including ImplicitOpenidGrant, AuthCodeOpenidGrant, and AbstractAuthorizeGrant along with all the related response types. However, it will require significant changes to this project.

@Sephster It may make sense to create a LTS branch and move faster forward for those who require new features. Authentication is changing fast right now. We need to keep up.