solid / authentication-panel

GitHub repository for the Solid Authentication Panel
MIT License
11 stars 15 forks source link

Client Registration does not state which entity it's optional for #56

Closed jaxoncreed closed 3 years ago

jaxoncreed commented 4 years ago

Under proof of Identity:

Client registration (static or dynamic) is not required in Solid-OIDC, thus Access Tokens need to be bound to the client, to prevent token replay attacks. This is achieved by using a Distributed Proof-of-Possession (DPoP) mechanism at the application layer.

In practice, we need to know if registration is optional for the client or the IdP. If it is optional for the client, then the IdP MUST support both a flow with registration and without registration to accommodate clients that do not use a registration flow and clients that do. The same is true vice versa.

Furthermore, it was my understanding that this spec was designed to follow the implementation built by Inrupt, unless something has changed, I believe that implementation required at least dynamic client registration.

jaxoncreed commented 4 years ago

Additionally later in the document, this recommendation is given:

Implementors SHOULD expire client IDs that are kept in server storage mitigate the potential for a bad actor to fill server storage with unexpired or otherwise useless client IDs.

Knowing that client Ids only apply to flows that involve registration, it seems oddly positioned. As read it feels like this is something global to the spec. I think it would fit better under the registration section in a new subheading devoted to "considerations if you do happen to allow registration"

jaxoncreed commented 3 years ago

Another interesting thing about making dynamic registration optional:

If you go to this link:

https://owntech.de/login?response_type=&display=&scope=&client_id=&redirect_uri=https://google.com

You'll get redirected to https://google.com. The reason for this is because this link is saying "log in as google.com"

elf-pavlik commented 3 years ago

@jaxoncreed could you please clarify what exactly dynamic registration changes here? Would it just require link above to have matching pair of client_id and redirect_uri or something else? For most public clients person creating that link could still have the client perform the dynamic registration and get client_id which would match redirect_uri.

jaxoncreed commented 3 years ago

Dynamic registration would require an AJAX request to register the application, then you'd put the clientId in the auth request. That way, it wouldn't be possible to trigger the login flow in just one link.

elf-pavlik commented 3 years ago

@jaxoncreed what do you think about adding clarification that the client needs to discover if the OP supports dynamic registration, by checking presence of registration_endpoint in OpenID Provider Metadata

elf-pavlik commented 3 years ago

Given https://solid.github.io/authentication-panel/solid-oidc/#clientids-oidc

If the Client does not use a WebID as the client identifier, then it MUST present a client identifier registered with the IdP via either OIDC dynamic or static registration.

It suggests that server MUST support at least dynamic registration since clients not using WebID as client identifier would need to register. Also @acoburn points out that OIDC requires servers to implement dynamic registration.

acoburn commented 3 years ago

OIDC requires some form of client registration on the server, whether that's static or dynamic. Otherwise, the server wouldn't be especially useful. Dynamic registration is optional under OIDC and support for that on the server can be discovered via standard OIDC discovery.

acoburn commented 3 years ago

As requirements for dynamic and static registration are defined by the OIDC specification, and it is possible for a client to discover which of these features an OIDC Authorization server supports, this issue is out of scope for the specification. If there is further discussion to be had on this issue, please re-open.