Skip to content
This repository has been archived by the owner on Jun 12, 2021. It is now read-only.

Client Registration issue with Bearer Header/Body #54

Open
angelakis opened this issue Apr 29, 2020 · 9 comments
Open

Client Registration issue with Bearer Header/Body #54

angelakis opened this issue Apr 29, 2020 · 9 comments

Comments

@angelakis
Copy link
Contributor

I believe that the client registration should be able to use the BearerHeader client_authn_method for simple authentication.

However, if configured to use it, it never checks the token as there is no get_client_id_from_token method implemented in oidcendpoint/oidc/registration.py. The method is called here

auth_info["client_id"] = get_client_id_from_token(
.

As a result it returns "" and no exception is raised.

I tried implementing a get_client_id_from_token method similarly to userinfo's one, but then there's a problem with unauthenticated registration as the No token exception is raised, because it cannot find a client_id in the request (correctly) and there is a get_client_id_from_token implemented. I think the last check should be corrected.

elif not client_id and get_client_id_from_token:

@rohe
Copy link
Contributor

rohe commented May 14, 2020

What does the token contain ?
Since this is at the registration endpoint, the client is not registered so there can be no reference to client information (like client_id/client_secret or keys).

@angelakis
Copy link
Contributor Author

Yeah, this is why I would like to use Bearer header token, so that a user agent can register a new client.

@rohe
Copy link
Contributor

rohe commented May 14, 2020

What kind of token and what does it contain if anything ?

@angelakis
Copy link
Contributor Author

Just an ordinary access token issued through another client.

@rohe
Copy link
Contributor

rohe commented May 14, 2020

What ? Do I get this right ?
You plan to use an access token assigned to another client (let's call him A) to be used as an authentication token by a new, un-registered client (B).
If B is to use it, it MUST be in the aud list (if it's a JWT token). But it can't because it doesn't have a name yet ?!

@angelakis
Copy link
Contributor Author

So let's say we have a client A and user X. We are the OP, we trust user X and we would like to allow her to register a new client B.

So user X goes to client A and authenticates with us. She then (through A, who supports this functionality) registers the new client B. So A issued a POST to us, using Bearer ******** as an authorization header, where ******** the access token for X.

I can already achieve this by writing a custom authentication class:

class CustomAuth(ClientAuthnMethod):
    tag = 'customauth'

    def is_usable(self, request=None, authorization_info=None):
        return(
            authorization_info is not None
            and authorization_info.startswith("Bearer ")
        )

    def verify(self, authorization_info, **kwargs):
        import pdb;pdb.set_trace()
        token = authorization_info[len("Bearer "):]
        _sdb = self.endpoint_context.sdb

        if not _sdb.is_token_valid(token):
            raise AuthnFailure()

        session = _sdb.read(token)
        if (session["authn_event"]["valid_until"] <= time_sans_frac()
            and "offline_access" not in session["authn_req"]["scope"]
        ):
            raise AuthnFailure()

        # info = collect_user_info(self.endpoint_context, session)

        return {"client_id": session['client_id']}

To make the use case clearer we could extend this to use the user info attributes and check if a user's specific claim/attribute has the value we want, so only then we allow the registration of a new client.

@angelakis
Copy link
Contributor Author

That's what I have in mind at least, I hope it makes (a bit) sense.

@rohe
Copy link
Contributor

rohe commented Sep 7, 2020

I don't think we reached an agreement on this issue.
First as you said you could implement this functionality by writing a custom authentication class and obviously you have a use case.
The question then is whether this is something we want in the core distribution or not ?

@peppelinux
Copy link
Member

peppelinux commented Nov 28, 2020

@angelakis please let us know if you can continue this thread, feel free to close It, I'm not aware if It Is something that would drive us to a conclusion. Share your thought if possible

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants