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

token introspection #649

Open
wants to merge 31 commits into
base: master
Choose a base branch
from
Open

token introspection #649

wants to merge 31 commits into from

Conversation

johakoch
Copy link
Collaborator

@johakoch johakoch commented Dec 21, 2022

JWT: call token introspection endpoint

jwt "..." {
  # ...

  beta_introspection {
    endpoint = "..." # URL of introspection endpoint
    backend = "..." # OR backend {...} for introspection request

    # for authenticating the client at the authorization server
    # see e.g. https://docs.couper.io/configuration/block/oauth2
    endpoint_auth_method = "..."
    jwt_signing_profile {
      # ...
    }
    client_id = "..."
    client_secret = "..."

    ttl = "..." # duration; positive value: cache introspection response, otherwise do not cache
  }
}

Reviewer checklist
  • Read PR description: a summary about the changes is required
  • Changelog updated
  • Documentation: docs/{Reference, Cli, ...}, Docker and cli help/usage
  • Pulled branch, manually tested
  • Verified requirements are met
  • Reviewed the code
  • Reviewed the related tests

@johakoch
Copy link
Collaborator Author

Do we have to lock whenever a new introspection request is created until the response is stored/cached (and so pausing introspection of the same token used in other client requests)?
Is it possible to lock per key?

accesscontrol/jwt.go Outdated Show resolved Hide resolved
@johakoch johakoch force-pushed the token-introspection branch 2 times, most recently from 409a314 to 11a5849 Compare January 31, 2024 08:40
@johakoch
Copy link
Collaborator Author

@malud https://codeclimate.com/github/coupergateway/couper/pull/649 I did my best with refactoring to reduce the number of return statements. But if there are several things to validate, max 4 return statements is not always possible.

Is there a way to ignore a specific check for a specific function?

@malud
Copy link
Collaborator

malud commented Jan 31, 2024

@malud https://codeclimate.com/github/coupergateway/couper/pull/649 I did my best with refactoring to reduce the number of return statements. But if there are several things to validate, max 4 return statements is not always possible.

Is there a way to ignore a specific check for a specific function?

You could extract the argument validations to own methods.

@johakoch
Copy link
Collaborator Author

You could extract the argument validations to own methods.

If there are at most 3 validations, yes.
With 4 validations, you have 4 possible errors + 1 happy path = 5 return statements.

And I don't think it makes much sense to group them by 2:

func validate() error {
  if err := validate12(); err != nil {
    return err
  }
  if err := validate34(); err != nil {
    return err
  }
  return nil
}

func validate12() error {
  if err := validate1(); err != nil {
    return err
  }
  if err := validate2(); err != nil {
    return err
  }
  return nil
}

func validate34() error {
  if err := validate3(); err != nil {
    return err
  }
  if err := validate4(); err != nil {
    return err
  }
  return nil
}

only to please codeclimate.

@malud
Copy link
Collaborator

malud commented Feb 1, 2024

I meant per type in new CSJ/PKJ methods. I will push a refactor (test) commit, otherwise we could increase the settings to 5 :) if possible.

@johakoch
Copy link
Collaborator Author

johakoch commented Feb 1, 2024

requestIntrospection() still has 4 error cases (roundtrip error, wrong status code, read error, JSON parse error) and the happy path return. To extract two of the error cases into a separate function, so that requestIntrospection() only has 4 return statements, would be artificial, IMO.

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

Successfully merging this pull request may close these issues.

let JWT validation call token instrospection
2 participants