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

why scope set to dummy for authorization_code and refresh_token #259

Open
gravypower opened this issue Sep 8, 2023 · 7 comments
Open

why scope set to dummy for authorization_code and refresh_token #259

gravypower opened this issue Sep 8, 2023 · 7 comments

Comments

@gravypower
Copy link

Summary

I am wondering why scope is set to dummy for authorization_code and refresh_token

should this also just repeat back the requests scopes like client_credentials does?

@nulltoken
Copy link
Contributor

@gravypower 👋 Sorry for the delay in getting back to you.

Could you please share a bit of context with regards to your question? Are you facing an issue or a possibly unexpected behavior?

@gravypower
Copy link
Author

thanks for responding @nulltoken. I am wanting to generate a JWT with scopes using the auth code flow. I expected that this library would just echo the scopes back like what happens with the client id and audience but this is not the case. I am working on the logic that applies authorisation in an app but was not able to do this with out forking the code base and removing where the scope is set to dummy.

I think doing this was correct but wanted to understand why dummy was set in the first place.

@nulltoken
Copy link
Contributor

nulltoken commented Oct 4, 2023

@gravypower Hmmm. I'm a little bit puzzled.

The code below defines an xfn transformer that will set the sub and amr and copy along the scope properties in the token payload to be generated.

xfn = (_header, payload) => {
Object.assign(payload, {
sub: 'johndoe',
amr: ['pwd'],
scope,
});
};

This transformer is later passed to buildToken

const token = await this.buildToken(req, tokenTtl, xfn);

This test showcases how an passed in scope is retrieved in the decoded token payload

https://github.com/axa-group/oauth2-mock-server/blob/master/test/oauth2-service.test.ts#L543-L573

In the end, I'm not really sure what doesn't work as you would expect and why you had to fork the code base.

Could you please help me understand better the problem you're facing?
(Or if it's quicker for you, a failing unit test showing what should work and doesn't)

@gravypower
Copy link
Author

gravypower commented Oct 5, 2023

I have altered that test you linked me to show the issue, see how when the grant type is authorization_code the scope is always dummy.

I think the issue is here:

it('should allow customizing the token response through a beforeTokenSigning event authorization_code', async () => {
    service.once('beforeTokenSigning', (token, req) => {
      expect(req).toBeInstanceOf(IncomingMessage);
      token.payload.custom_header = req.headers['custom-header'];
      token.payload.iss = "https://tada.com";
    });

    const res = await tokenRequest(service.requestHandler)
      .set('Custom-Header', 'custom-token-value')
      .send({
        grant_type: 'authorization_code',
        scope: 'a-test-scope',
      })
      .expect(200);

    const key = service.issuer.keys.get('test-rs256-key');
    expect(key).not.toBeNull();

    expect(res.body).toMatchObject({
      access_token: expect.any(String),
    });
    const resBody = res.body as { access_token: string };

    const decoded = await verifyTokenWithKey(service.issuer, resBody.access_token, 'test-rs256-key');

    expect(decoded.payload).toMatchObject({
      iss: "https://tada.com",
      scope: 'a-test-scope',
      custom_header: 'custom-token-value',
    });
  });

@gravypower
Copy link
Author

I am happy to raise an PR to address this, just wanted to know if this was done for a reason?

@nulltoken
Copy link
Contributor

just wanted to know if this was done for a reason?

paging @poveden that may have a better view on this

@poveden
Copy link
Contributor

poveden commented Nov 6, 2023

Hi @gravypower this was introduced about 4 years ago. I think (if my memory serves me well) that it was added because some client library complained about them missing. Again, this was 4 years ago, so probably this is no longer the case today.

So, yes, please, raise a PR. Thanks!

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

No branches or pull requests

3 participants