node-solid-server icon indicating copy to clipboard operation
node-solid-server copied to clipboard

"scopes_supported": ["webid"]

Open michielbdejong opened this issue 4 years ago • 14 comments

According to https://solid.github.io/solid-oidc/#discovery the JSON on /.well-known/openid-configuration must include at least "scopes_supported": ["webid"] and if you combine https://solid.github.io/solid-oidc/#tokens-access with https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata you can conclude that "claims_supported": ["webid"] is also at least recommended. The new solid-oidc-tests test for this, and so NSS instances will fail both these tests. See also https://gitter.im/solid/test-suite?at=6193afa5a41fd20699464377

michielbdejong avatar Nov 16 '21 13:11 michielbdejong

@michielbdejong should this be closed following https://github.com/solid/solid-oidc/issues/56

bourgeoa avatar Nov 16 '21 21:11 bourgeoa

No, that issue was just about whether the current text of the solid-oidc spec is clear enough. The need/desirability for adding them in all Solid-oidc servers is undisputed, that stems from the 'recommended' mention in the parent oidc spec.

So NSS should announce both scopes_supported and claims_supported.

michielbdejong avatar Nov 17 '21 09:11 michielbdejong

I do not follow recommended is not a MUST but a MAY.

bourgeoa avatar Nov 17 '21 13:11 bourgeoa

In point of fact, RECOMMENDED is neither a MUST nor a MAY, but a SHOULD.

Quoted from RFC 2119:

3. SHOULD   This word, or the adjective "RECOMMENDED", mean that there
   may exist valid reasons in particular circumstances to ignore a
   particular item, but the full implications must be understood and
   carefully weighed before choosing a different course.

TallTed avatar Nov 17 '21 15:11 TallTed

@acoburn Could you clarify this point.

It has got some importance due to a need to add scopes_supported: ["openid", "offline_access"] to be able to use latest https://github.com/inrupt/solid-client-authn see issue https://github.com/inrupt/solid-client-authn-js/issues/1991 and solution provided by inrupt https://gitter.im/solid/solidos?at=621cca8d6b912423200f5e4f

What should be implemented with the intention to follow solid-oidc spec and not to break NSS using inrupt auth module.

bourgeoa avatar Feb 28 '22 13:02 bourgeoa

see https://github.com/solid/oidc-op/pull/35

bourgeoa avatar Feb 28 '22 14:02 bourgeoa

Just to clarify, I had the configuration wrong when I said scopes_supported be set to include "webid" in the array; It didn't seem to actively break anything when I tested it with @theRealImy, but on closer look, as NSS doesn't actually support the latest https://solid.github.io/solid-oidc/ spec, which adds the "webid" scope. Having an NSS server advertise it supports the "webid" scope when it doesn't support the latest solid-oidc spec would be bad.

I've also linked other issues in that are all about supporting the latest WebID / solid-oidc spec.

ThisIsMissEm avatar Feb 28 '22 14:02 ThisIsMissEm

I would very much reiterate what @ThisIsMissEm wrote above.

If NSS intends to implement support for the draft Solid-OIDC specification, then it needs to add the webid scope to the OpenID metadata in scopes_supported. Adding that scope effectively communicates to clients that "this server supports the draft Solid-OIDC spec". This value is a crucial part of the discovery mechanism for Solid-OIDC.

If NSS does not support the draft spec, then please do not add the webid scope to that array. Doing so will only confuse clients. That would be akin to sending a response header with Accept-Patch: text/n3 but not actually supporting N3 patch.

acoburn avatar Mar 03 '22 18:03 acoburn

Thanks @acoburn Not understanding your comments on https://github.com/solid/oidc-op/pull/35 I will remove webid from scopes and claims. If NSS does not support draft solid-oidc can you explain why does NSS need to add 'scopes_supported: ["openid", "offline_access"]' to provider.json

bourgeoa avatar Mar 03 '22 20:03 bourgeoa

Hey @bourgeoa, that came from me, so let me explain it a bit: in @inrupt/solid-client-authn we'd have code that mean we accidentally expected servers to allways be returning scopes_supported, we couldn't quickly release a fix, so the easier option was to modify the existing deployments of NSS to advertise the scopes they support.

in general, clients will use both scopes_supported, claims_supported and other data in the /.well-known/* files to negotiate with servers as to their capabilities; NSS did not, and only used the older spec feature of solid_oidc_supported which is non-standard, so having NSS add scopes_supported will improve NSS's interoperability with not just Inrupt's SDK, but all client implementations.

Hope that clears things up a bit?

ThisIsMissEm avatar Mar 03 '22 21:03 ThisIsMissEm

@ThisIsMissEm Thanks, finally it is clearer. Adding scopes_supported: ["openid", "offline_access"] will improve compatibility with OIDC client and among them Inrupt's. This will not imply that NSS supports the draft Solid-OIDC specification.

bourgeoa avatar Mar 03 '22 21:03 bourgeoa

@bourgeoa correct! If you then wanted to support the draft Solid-OIDC spec, you'd then add "webid" to that list, and probably have to add the key "dpop_signing_alg_values_supported" (though I'm not super versed in how dpop works off top of head)

ThisIsMissEm avatar Mar 05 '22 18:03 ThisIsMissEm

@ThisIsMissEm for information PR #1675 add scopes supported on server creation and #1673 add oidcIssuer in default profile on pod creation.

If draft Solid-OIDC is approved I shall look at scopes webid. I will need more explanation as to what is needed.

bourgeoa avatar Mar 05 '22 20:03 bourgeoa