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

Support of resource parameter when a connector request an access token from an IDP #4668

Open
scandinave opened this issue Dec 11, 2024 · 7 comments · May be fixed by #4680
Open

Support of resource parameter when a connector request an access token from an IDP #4668

scandinave opened this issue Dec 11, 2024 · 7 comments · May be fixed by #4680
Labels
enhancement New feature or request

Comments

@scandinave
Copy link
Contributor

Feature Request

Which Areas Would Be Affected?

IAM/Oauth2

Why Is the Feature Desired?

A resource parameter, as defined by the RFC 8707, allows a client (EDC) to request an access token to its IDP by specify the aud claim that should be added to the token.Without this parameter the IDP have no mean to restrict the usage of the token to a specific EDC. In a Dataspace where multiples organisations manages their connectors the missing of this feature can allow a organisation to use an access token to target another EDC that it was unintended for.

Solution Proposal

Update the org.eclipse.edc.iam.oauth2.identity.Oauth2ServiceImpl to add this parameter with a value corresponding to the target EDC URL.

@github-actions github-actions bot added the triage all new issues awaiting classification label Dec 11, 2024
@jimmarino
Copy link
Contributor

Can you be more specific about what this issue refers to? The aud claim must be set in the protocol layer by AudienceResolver.

@scandinave
Copy link
Contributor Author

My use case is that I have a dataspace with multiple EDC connector managed by differents enterprises. I don't want that an enterprise is able to use an access token retrieved by its connector onto all others connectors in the dataspace.

The RFC 8707 state that an OAuth2 client can use a resource parameter which is a URL, to indicate to the IDP the target resource server on which the access token will be used. This allows the IDP to add this target as audience inside the access token.

Without this feature, the IDP cannot know the target audience, so it either means that a common audience is used for all connectors in the same dataspace or the audience is not set and can not be checked. Both are not good in term of security.

I think it should be possible to add this parameter as part of the access token request considering the calling EDC known the called EDC. The value of the parameter is this case would be the EDC host URL.

If you consider this issue ok. We can work on it and make a PR.

@paullatzelsperger
Copy link
Member

paullatzelsperger commented Dec 11, 2024

@scandinave just to be 100% sure: you are proposing to add another property to the request in the OAuth2ServiceImpl like so:

@NotNull
private Oauth2CredentialsRequest createRequest(TokenParameters parameters, String assertion) {
    return PrivateKeyOauth2CredentialsRequest.Builder.newInstance()
                .url(tokenUrl)
                .clientAssertion(assertion)
                .scope(parameters.getStringClaim(JwtRegisteredClaimNames.SCOPE))
                .resource(parameters.getStringClaim(JwtRegisteredClaimNames.AUDIENCE)) // <-- this is new!
                .grantType(GRANT_TYPE)
                .build();
}

correct? The parameters object already contains that value, because it's extracted from the remote message using the AudienceResolver, as @jimmarino said. So it should be an easy fix.

However, be advised that going forward, the EDC project will focus solely on the Decentralized Claims Protocol as its main identity subsystem (i.e. the IdentityAndTrustService). This means that there is a high chance that OAuth2 will be deprecated and ultimately discontinued. In that case, once it is deprecated, we are looking at ca. 3 months until final removal.

We therefore strongly recommend considering a switch over to DCP.

@scandinave
Copy link
Contributor Author

scandinave commented Dec 11, 2024

Yes, this is something like this, if parameters.getStringClaim(JwtRegisteredClaimNames.AUDIENCE) can return a URL as required by the specification.

We currently have a dataspace that use the old centralized model for its security, and we need to address this issue quickly.
We have chosen this model because of assumption that state that DCP was not production ready yet. ( here, here and here ).

Do you mean that DCP is now considered production ready? If not, do you have provisional roadmap on it? This something we definitely have in our backlog.

@paullatzelsperger
Copy link
Member

paullatzelsperger commented Dec 12, 2024

parameters.getStringClaim(JwtRegisteredClaimNames.AUDIENCE) will contain whatever the AudienceResolver implementation stores there. In the case of OAuth2, this is the counterPartyAddress of the message receiver by default and unless another audience resolver is registered, check here.

As for DCP, you can safely disregard the first link. That clearly is a remnant of times passed, e.g. it still contains references to ION which was a blockchain-based DID method.

The DCP specification is slated for an "Editor's Draft" release in December and the implementation of the Credential Issuance Protocol in IdentityHub will likely come in 2025. DCP is actively used in production in several dataspaces, e.g. Tractus-X.

I understand the need for a quick solution, and that is fine, but it is something to keep in mind. You can also checkout the MVD project if you haven't, to play around with DCP a bit.

@scandinave
Copy link
Contributor Author

Thanks for this precious information. Can we make a PR for this little fix to allow us to resolve our problematic until we schedule the migration to DCP ?

@paullatzelsperger
Copy link
Member

Can we make a PR for this little fix to allow us to resolve our problematic until we schedule the migration to DCP ?

sure, go ahead!

@ndr-brt ndr-brt added enhancement New feature or request and removed triage all new issues awaiting classification labels Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants