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

Handle more than one client secret per client #1712

Closed
DennisPattmann5012 opened this issue Jan 28, 2020 · 8 comments
Closed

Handle more than one client secret per client #1712

DennisPattmann5012 opened this issue Jan 28, 2020 · 8 comments
Labels
rfc A request for comments to discuss and share ideas. stale Feedback from one or more authors is required to proceed.
Milestone

Comments

@DennisPattmann5012
Copy link
Contributor

Is your feature request related to a problem? Please describe.

If we want to rotate a client secret for a specific client, there is no way to have two client secrets in place for a specific time. Without this we have a synchronous dependency between client secret rotation and oauth client modification.

Describe the solution you'd like

We have the idea of adding a client secret struct like this:

type clientSecret struct {
    clientSecretId: string,
    clientSecret:   string,
    createdAt:      time.Time(),
    validFrom:      time.Time(),
    validUntil:     time.Time(),
}

and pass this struct to client:

type Client struct {
...
clientSecrets: [2]clientSecret
...
}

Describe alternatives you've considered

We have two other ideas, we thought of:

  1. Adding a second client_secret directly to the client struct
    We dislike this idea because we were missing the secret metadata (like validFrom and validUntil)
  2. Make clientSecret in client to a slice of string
    Rotation is nearly impossible because we wont know if a client secret is still in use

Additional context

We would love to work on a PR for that after we discussed the design of the feature

@aeneasr
Copy link
Member

aeneasr commented Jan 28, 2020

I don't think this would be compliant with OpenID Connect Dynamic Client Registration, right?

@DennisPattmann5012
Copy link
Contributor Author

Didn't had this in mind.

What do you think about a new field like rotated_client_secrets that includes a client_secret struct? May you have some ideas?

@aeneasr aeneasr added feat New feature or request. breaking change Changes behavior in a breaking manner. rfc A request for comments to discuss and share ideas. and removed breaking change Changes behavior in a breaking manner. feat New feature or request. labels Aug 21, 2020
@aeneasr aeneasr added this to the unplanned milestone Aug 21, 2020
@aeneasr
Copy link
Member

aeneasr commented Sep 5, 2021

Sorry for not replying. This is possible when using JSON Web Keys (private_key_jwt auth), as you can register more than one JWK per client. It is an advanced flow, but it’s also an advanced requirement :)

For secrets there is no standardized option and due to low community interest in the issue I am closing this issue, see reasoning below :)

I am closing this issue as it has not received any engagement from the community or maintainers in a long time. That does not imply that the issue has no merit. If you feel strongly about this issue

  • open a PR referencing and resolving the issue;
  • leave a comment on it and discuss ideas how you could contribute towards resolving it;
  • open a new issue with updated details and a plan on resolving the issue.

We are cleaning up issues every now and then, primarily to keep the 4000+ issues in our backlog in check and to prevent maintainer burnout. Burnout in open source maintainership is a widespread and serious issue. It can lead to severe personal and health issues as well as enabling catastrophic attack vectors.

Thank you to anyone who participated in the issue! 🙏✌️

@aeneasr aeneasr closed this as completed Sep 5, 2021
@aeneasr
Copy link
Member

aeneasr commented Sep 5, 2021

Sorry, actually I noticed that we support this now in fosite. So it would be little work to add it here, if anyone is up for the challenge. First it would need an API proposal though!

@aeneasr aeneasr reopened this Apr 11, 2022
@dpattmann
Copy link

Hi @aeneasr,

I'm still interested in this feature and would like to work on it.

About the API proposal, I'm not sure if it's necessary to touch the API, at least in a first version.

My idea is to update the client object/database to hold RotatedSecrets and add a new configuration option like SecretRotion : n for max n secrets. Default would be 0 to disable multiple secrets.

When a client is updated via PUT|PATCH /clients{id} and RotatedSecrets is greater 0 the old secret will be moved to SecretRotion.

What's your opinion?

@developerDemetri
Copy link

Hi, our team is also interested in this feature to enable graceful client secret rotations on a regular cadence.

My current Go knowledge is limited and I'm just starting to ramp up on Hydra. Probably not able to contribute quality features for a bit, but happy to help with this once I'm ramped up.

@github-actions
Copy link

Hello contributors!

I am marking this issue as stale as it has not received any engagement from the community or maintainers for a year. That does not imply that the issue has no merit! If you feel strongly about this issue

  • open a PR referencing and resolving the issue;
  • leave a comment on it and discuss ideas on how you could contribute towards resolving it;
  • leave a comment and describe in detail why this issue is critical for your use case;
  • open a new issue with updated details and a plan for resolving the issue.

Throughout its lifetime, Ory has received over 10.000 issues and PRs. To sustain that growth, we need to prioritize and focus on issues that are important to the community. A good indication of importance, and thus priority, is activity on a topic.

Unfortunately, burnout has become a topic of concern amongst open-source projects.

It can lead to severe personal and health issues as well as opening catastrophic attack vectors.

The motivation for this automation is to help prioritize issues in the backlog and not ignore, reject, or belittle anyone.

If this issue was marked as stale erroneously you can exempt it by adding the backlog label, assigning someone, or setting a milestone for it.

Thank you for your understanding and to anyone who participated in the conversation! And as written above, please do participate in the conversation if this topic is important to you!

Thank you 🙏✌️

@github-actions github-actions bot added the stale Feedback from one or more authors is required to proceed. label Aug 17, 2023
@zepatrik zepatrik removed the stale Feedback from one or more authors is required to proceed. label Sep 6, 2023
Copy link

github-actions bot commented Sep 6, 2024

Hello contributors!

I am marking this issue as stale as it has not received any engagement from the community or maintainers for a year. That does not imply that the issue has no merit! If you feel strongly about this issue

  • open a PR referencing and resolving the issue;
  • leave a comment on it and discuss ideas on how you could contribute towards resolving it;
  • leave a comment and describe in detail why this issue is critical for your use case;
  • open a new issue with updated details and a plan for resolving the issue.

Throughout its lifetime, Ory has received over 10.000 issues and PRs. To sustain that growth, we need to prioritize and focus on issues that are important to the community. A good indication of importance, and thus priority, is activity on a topic.

Unfortunately, burnout has become a topic of concern amongst open-source projects.

It can lead to severe personal and health issues as well as opening catastrophic attack vectors.

The motivation for this automation is to help prioritize issues in the backlog and not ignore, reject, or belittle anyone.

If this issue was marked as stale erroneously you can exempt it by adding the backlog label, assigning someone, or setting a milestone for it.

Thank you for your understanding and to anyone who participated in the conversation! And as written above, please do participate in the conversation if this topic is important to you!

Thank you 🙏✌️

@github-actions github-actions bot added the stale Feedback from one or more authors is required to proceed. label Sep 6, 2024
@github-actions github-actions bot closed this as completed Oct 6, 2024
@zepatrik zepatrik reopened this Nov 8, 2024
@github-actions github-actions bot closed this as completed Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rfc A request for comments to discuss and share ideas. stale Feedback from one or more authors is required to proceed.
Projects
None yet
Development

No branches or pull requests

5 participants