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

feat(auth): public list authentication methods endpoint #1240

Merged
merged 6 commits into from Dec 22, 2022

Conversation

GeorgeMac
Copy link
Contributor

@GeorgeMac GeorgeMac commented Dec 22, 2022

Fixes FLI-120

Implements the /auth/v1/method endpoint.
This endpoint lists metadata about which authentication methods are configured and in what way.

I have first introduced a small refactor to the config/authentication related config.
This allows for simple registering of new methods, while ensuring they are then appropriately reflected in the API without additional changes.

I also added some cases to the e2e test, which invokes this endpoint to ensure it's mounted.
And another which attempts to use the Cookie header to authenticate a request when OIDC is enabled.

curl http://localhost:8080/auth/v1/method | jq .                                                                                                                                              
{
  "methods": [
    {
      "method": "METHOD_TOKEN",
      "enabled": false,
      "sessionCompatible": false
    },
    {
      "method": "METHOD_OIDC",
      "enabled": false,
      "sessionCompatible": true
    }
  ]
}

@GeorgeMac GeorgeMac requested a review from a team as a code owner December 22, 2022 14:41
}

for _, info := range conf.Methods.AllMethods() {
server.resp.Methods = append(server.resp.Methods, &auth.MethodInfo{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you think its any kind of security risk to publicize all the methods, even if they arent enabled? I could potentially see a situation where an attacker could leverage this information somehow (maybe?) create an exploit?

do you think its worth only returning enabled methods?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i mean i guess since its open source, and has public docs that we arent really able to hide which methods are available

@markphelps
Copy link
Collaborator

we'll need to update https://github.com/flipt-io/flipt-api once we release

@GeorgeMac GeorgeMac merged commit 9fe5a88 into main Dec 22, 2022
@GeorgeMac GeorgeMac deleted the gm/list-methods branch December 22, 2022 15:15
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.

None yet

2 participants