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

Origin is validated before credentials in auth process #1568

Closed
2 tasks
orenbm opened this issue May 25, 2020 · 2 comments · Fixed by #1564
Closed
2 tasks

Origin is validated before credentials in auth process #1568

orenbm opened this issue May 25, 2020 · 2 comments · Fixed by #1564
Assignees

Comments

@orenbm
Copy link
Member

orenbm commented May 25, 2020

At current, our authentication process is as follows (in high level):

  • validate_authenticator_exists: validate that the authenticator is implemented in Conjur
  • validate_security: validate that the authenticator is enables, it has a defined webservice, the user exists and that it has authenticate permissions on the webservice
  • validate_credentials: validate that the given credentials are valid (e.g api_key is correct)
  • validate_origin: validate that the user can authenticate from the IP that the request was sent (i.e using the restricted_to policy entity).
  • audit_success: write a success message to the audit log
    new_token: create a new Conjur access token

We should replace the order from:

validate_security
validate_credentials
validate_origin

to:

validate_security
validate_origin
validate_credentials

the security and origin validation are similar as in both we verify that the user can authenticate with Conjur. So it makes sense to first validate that the user can authenticate with Conjur, before we actually authenticate it.

This change will improve the readability of our code as its logic will make more sense.
Furthermore, the origin validation is very quick so we don't want to fail on it after we perform heavy validations of the credentials.

DoD:

  • origin is validated before credentials
  • existing tests still pass (no additional tests are required)
@orenbm orenbm self-assigned this May 25, 2020
@orenbm orenbm added this to the PalmTree - sprint 2011 milestone May 25, 2020
@Tovli Tovli linked a pull request May 26, 2020 that will close this issue
@Tovli
Copy link
Contributor

Tovli commented May 26, 2020

I feel like I missing something in this issue.
What do you mean by "first validate that the user can authenticate with Conjur, before we actually authenticate it."

The authentication happen only after all the checks had passed, regardless of their order
What do we gain here?

@orenbm
Copy link
Member Author

orenbm commented May 26, 2020

what i mean is that we have 2 steps:

  1. Validate that the user can authenticate with the authenticator (i.e has the permissions)
  2. validate_security
  3. validate_origin
  4. Validate the user's credentials with the authenticator
  5. validate_credentials

What i'm trying to do here is stack the validations in that order. The current validations are security -> credentials -> origin. I want to change it to security -> origin -> credentials to maintain the logic described above.

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

Successfully merging a pull request may close this issue.

2 participants