Vault: OpenID Connect authentication method

Created on 7 Sep 2018  路  37Comments  路  Source: hashicorp/vault

I've made an implementation here: https://github.com/postmates/vault-plugin-oauth/pull/1

This implements a full OAuth 2.0 client within Vault. This has positive security implications as Vault is then able to authenticate with the OAuth server with a client secret. This prevents a wide range of attacks that involve an attacker creating a malicious OAuth client which receives tokens which can then be used to authenticate to Vault.

It also implements all the UI necessary for a user to authenticate. To the Vault user, it looks no different than using OAuth to authenticate to any website, except the process is initiated on the command-line rather than clicking a link:

  1. Run vault-login-oauth
  2. A browser opens with a login form. Fill it out.

Assuming everything goes well the token is automatically saved to ~/.vault-token and the user is now authenticated.

It supports any OpenID Connect provider, including:

I'd like to request this plugin gets included in Vault so that:

  • More eyes are on it. More eyes means less bugs and better security.
  • The vault-login-oauth functionality can be included as part of vault login so Vault administrators don't have to distribute extra tools to their users.
  • The current Okta, JWT/OIDC, and GitHub methods can be replaced by a more secure alternative.

Alternatives

Built-in JWT/OIDC method

This does not implement an OAuth client, so any UI to obtain the identity token is left to the Vault administrator to implement and distribute to Vault users. This is not only inconvenient, but these one-off implementations are likely to be sources of implementation errors on account of not having been well-reviewed. It's quite easy to run some curl commands and get a token, but much harder to do it securely.

The OAuth client secret has to be distributed to all Vault users, or flows that don't require a secret must be used. This makes a successful attack by a malicious client easier.

It's easy to misconfigure. Neglecting to bind the role to an appropriate audience or issuer means Vault will accept identity tokens it should not, for example an identity token a user used to authenticate with the same OAuth provider to a site run by 3rd party, like a discussion forum which could be compromised or malicious.

Has no mechanism for verifying arbitrary claims in the token. For example, Google's identity tokens will indicate if a user is a member of a managed G Suite domain. There's no way to allow users only from a particular domain.

Built-in Okta method

The Okta method requires trusting Vault with user credentials. Possibly acceptable, but less than ideal. Precludes the use of MFA methods that require interaction with the browser, like U2F.

Built-in GitHub method

(GitHub support isn't in the PR linked above, but I have a PoC implementation. If this is accepted I'll be glad to complete that effort.)

Like the JWT/OIDC method, obtaining a GitHub personal access token is left as an exercise for the user. These tokens can be obtained through some manual clicking through the GitHub web UI, but even so this workflow could be more usable.

Subject to the issue described in the documentation for this method:

IMPORTANT NOTE: Vault does not support an OAuth workflow to generate GitHub tokens, so does not act as a GitHub application. As a result, this method uses personal access tokens. An important consequence is that any valid GitHub access token with the read:org scope can be used for authentication. If such a token is stolen from a third party service, and the attacker is able to make network calls to Vault, they will be able to log in as the user that generated the access token. When using this method it is a good idea to ensure that access to Vault is restricted at a network level rather than public. If these risks are unacceptable to you, you should use a different method.

The trouble is a personal access token isn't scoped to any particular audience, so while a personal access token implies at some point the user authenticated, it doesn't prove the user is currently present or that they intended to authenticate to Vault.

This risk is mitigated with an OAuth integration, as the authentication code is the credential the user must provide to Vault. Authentication codes are valid only for a limited time, and only for a specific OAuth client.

https://github.com/hashicorp/vault/pull/2571

Uses the resource owner flow. Same issues as Okta method.

https://github.com/grapeshot/google-auth-vault-plugin

Supports Google only.

Built-in GCP method

Good for authenticating Google compute instances, but awkward for human users.

Most helpful comment

Funny, you had a lot of comments (mostly negative) a month ago. Or did you not actually read the PR? :thinking:

@audip status of the PR is "Functional, but unable to merge because @jefferai rejected it without reading it". Same status it's been in for over 2 months.

All 37 comments

@avoidik @mikeokner @rocktavious @leeavital @grunzwei may be interested in this, based on comments in other issues. If not sorry for the noise, unsubscribe button is to the right.

Given that the vast majority of the interesting logic happens in the CLI helper, is there a reason not to combine that with the JWT plugin? I took a quick look through the code and it doesn't bring that much extra logic to the table, and could reduce duplication of a bunch of the logic.

Not sure I understand what you mean. The CLI helper has to receive the authorization code from the OAuth server, which comes as an HTTP GET with the authorization code in the query parameters after the user has provided credentials to the identity provider. Unless I'm mistaken, a Vault plugin can't handle such a request, so we have to run an HTTP server somewhere to handle it. Are you proposing running that on the Vault server somehow?

I thought you were using a local listener via the CLI helper to receive it.

I am. So if vault-login-oauth (which runs on the Vault client) moves to the plugin (which runs on the Vault server), how would that continue to work?

@jefferai further thoughts?

I'm not suggesting moving that to the plugin, I'm suggesting moving the server side components.

How then would the OAuth client secret be secured?

I don't know what parameters you have and what does and doesn't need to be secured. know that in a previous conversation I suggested a design doc to get everyone aligned. I still think it's the right next step.

Did you see the documentation included with the linked PR?

As for what needs to be secured, RFC6819 is a good reference. Searching for "public client" you can get an idea of the range of attacks that are mitigated simply by having a private client, as proposed here.

RFC8252 describes best practices for native apps. Although this proposal isn't exactly a native app, the technique of getting the authorization code on a listener on 127.0.0.1 is the same idea.

RFC7636 describes PKCE, countermeasure for certain authorization code interception attacks of public clients, as the existing JWT/OIDC method would have to be in practice. You may find it useful to read the threats described therein to understand the kinds of things this proposal mitigates.

No, I didn't see the readme.

Okay, after looking at the readme, the flow diagram, and the config parameters, I still don't see a reason this functionality can't be added to the existing jwt plugin.

If by "this functionality" you mean something that looks the same from the user's perspective, sure. I could implement that in the existing JWT plugin.

However it would have to be a _public client_. As I've explained, and supported with ample documentation, a public client is not as secure as a private one. People do implement public clients, but only when a private client is not possible. The id_token is effectively a bearer token: if an attacker ever gets one the system is compromised since the attacker can now present it to Vault. Without a private client (meaning, one that can secure a client secret), the OAuth server can't authenticate the client, so it can't know if it might be giving tokens to an attacker. So instead it must rely on weaker mechanisms, which boil down to registering redirect URIs and trusting none of those _unauthenticated_ channels are ever compromised. That introduces more attack surface and opportunity for configuration error.

Given that this will be used to authenticate users to Vault, some of them administrators with access to the "keys to the kingdom" stored therein, I would think anyone would be glad to take all the security they can get. Do you disagree?

Apologies if I'm just not understanding what you are asking. Could I trouble you to write more than two sentences on your objection to the proposal, and just what it is you want? Feel free to comment on the PR, if an inline comment with context in the code will make things more clear.

I don't want to put words into @jefferai's mouth, but why not just extend the existing JWT plugin to also support the authorization code flow? It sort of already supports the implicit flow by expecting an id_token as the jwt paramemter, and it also already supports an OIDC discovery URL, so there's a good amount of overlap, and it would seem logical to extend it to also support the authorization code flow as well, rather than create a completely new backend. Unless there are reasons I'm not seeing.

I guess I'm just not sure how to explain it better, although maybe what Joel said clears it up for you.

You are proposing, in the repo, a CLI helper and a server plugin. I've been suggesting that the server side of your plugin -- IOW, not the CLI helper -- be merged into the JWT plugin. I don't really know how to be more clear about what I'm suggesting.

One other thought that I've been noodling on as well, now that the Vault UI is open-source, is putting the client redirection support into the UI. A user would browse to the Vault UI, click "login with my OIDC provider" (or whatever), get redirected to the OIDC provider, the OIDC provider would redirect back to the Vault UI, which would then send either the id_token or the authorization code back to the Vault server (depending on the flow desired), and then the user could get a Vault token back that way. Might be a nice complement to the CLI helper and get around the issue in the To do section of the README where "Some providers require an exact match on the redirect URI."

I initially tried extending the JWT plugin, but abandoned that approach pretty quickly. They really aren't all that similar.

Firstly, they consume entirely different inputs. The JWT plugin requires the user to provide a JWT. The OAuth plugin requires an authorization code. I'd need to introduce configuration to the JWT plugin to conditionally disable its capability to accept JWT, since if the OAuth client is implement in the Vault server there is no legitimate use case for this function.

The JWT plugin has configuration flexibility which is unnecessary for OAuth, and which if misconfigured, can result in an insecure system. bound_audience and bound_issuer are optional with the JWT plugin, but required with OAuth.

It sort of already supports the implicit flow by expecting an id_token as the jwt paramemter, and it also already supports an OIDC discovery URL, so there's a good amount of overlap,

Not really. The implicit flow provides the token in the URL fragment. Vault does not support accepting parameters this way, nor could it, because the URL fragment isn't part of the HTTP request. (Also note the implicit flow is not recommended by current best practice.)

Nor does Vault support any other kind of OAuth flow or even part of one, again because the parameter format required for a plugin is incompatible with the OAuth protocol.

Granted, it does support an OIDC discovery URL. But so does github.com/coreos/go-oidc, which also supports the rest of the OIDC protocol. So I wouldn't be reducing duplication so much as abandoning the library which does precisely what's needed, and then extending the JWT plugin to reimplement the parts of go-oidc which are missing.

I could attempt to integrate go-oidc with the JWT plugin, but as the JWT plugin allows things which are not valid in OIDC (such as not verifying the issuer) I'm not sure that's possible.

Furthermore the intention of the oauth plugin is to support not only OIDC, but other OAuth providers such as GitHub. It's written in such a way to make this easy, through the IdentityProvider interface. The only implementation in the PR is oidcprovider, but I also have a proof of concept GitHub provider which replaces the id_token verification logic with a call to GitHub's user API. In a case such as this the overlap with the JWT plugin is zero, except for things common to any auth plugin.

(The random port is the main issue with GitHub support -- I plan to address that by adding to the configuration a listen port, or maybe a short list of ports, for the HTTP server started by vault-login-oauth. Your idea of using the Vault UI is a good one, I'll investigate that as well.)

If duplication is a concern, I would instead propose stripping OIDC functionality from the JWT/OIDC plugin entirely. It's possible to continue using that plugin with JWTs obtained from OIDC if the keys are obtained manually, which isn't difficult, and certainly not more difficult than the OAuth client that would have had to been implemented by anyone using the plugin in that way. But it would be good to encourage users to migrate to the OAuth plugin for the security benefit, and it's likely if they've implemented custom tooling it can be simplified by stopping at the authorization code response and integrated with the OAuth plugin at that point instead.

Given this OAuth plugin could eventually also serve the use cases covered by the Okta and GitHub plugins, I think we're still looking at a substantial net reduction in lines of code.

Firstly, they consume entirely different inputs. The JWT plugin requires the user to provide a JWT. The OAuth plugin requires an authorization code. I'd need to introduce configuration to the JWT plugin to conditionally disable its capability to accept JWT, since if the OAuth client is implement in the Vault server there is no legitimate use case for this function.

This is quite trivial. For a given role, specify a type of the role. The necessary inputs are then gated on the type of the role. Not at all dissimilar to what the AWS secrets engine does.

The JWT plugin has configuration flexibility which is unnecessary for OAuth, and which if misconfigured, can result in an insecure system. bound_audience and bound_issuer are optional with the JWT plugin, but required with OAuth.

Same.

I could attempt to integrate go-oidc with the JWT plugin, but as the JWT plugin allows things which are not valid in OIDC (such as not verifying the issuer) I'm not sure that's possible.

But for a role of the right type, it'd always validate the issuer.

Furthermore the intention of the oauth plugin is to support not only OIDC, but other OAuth providers such as GitHub. It's written in such a way to make this easy, through the IdentityProvider interface. The only implementation in the PR is oidcprovider, but I also have a proof of concept GitHub provider which replaces the id_token verification logic with a call to GitHub's user API. In a case such as this the overlap with the JWT plugin is zero, except for things common to any auth plugin.

I realize you're looking at this with an eye for a single plugin, but when you have large swaths of functionality that end up being the same, fixes to one plugin don't tend to end up being pulled over into the other. We saw this with the proliferation of database plugins, which (even though somewhat of an extreme case) meant that we'd see old bugs in some backends because they'd only been ported/fixed to others.

Even something as basic as parsing a submitted role value -- if we change how it's parsed, and forget to change it in the other plugin, now we have a discrepancy. Those just keep adding up.

There are many different plugins in Vault that behave in relatively different ways based on configuration of the mount or configuration of the role, but where there is enough similarity that from a maintenance perspective it makes quite a lot of sense to keep them grouped. For AWS, it can generate IAM users or STS users, and these workflows are quite distinct, but by keeping them in the same plugin it ensures that as we fix items related to the actual AWS API calls, the AWS clients, and other common things all of those workflows benefit.

SSH is similar -- there isn't that much that is the same at the user level between certs, OTPs, and dynamic keys, but on the backend we share common bug fixes, SSH connection logic, etc.

That's really what I'm going for here. I don't at all disagree that "validate this JWT" and "help me through a full OIDC workflow" are quite dissimilar. But from a maintenance perspective, I look at that and actually see a lot of overlap: Value bindings, OIDC clients/configuration, validation key configuration, role parameter parsing/storing, renewal and revocation logic, ..., because in the end we still end up with a JWT, and we still want to validate/control access based on aspects of that JWT.

Straight-up non-JWT OAuth is actually where I see more dissimilarity on the server side, but as you noted on the client side it's pretty much the same, and besides the dissimilarity is there regardless of whether the JWT-based flows are in the same plugin or not.

Given this OAuth plugin could eventually also serve the use cases covered by the Okta and GitHub plugins, I think we're still looking at a substantial net reduction in lines of code.

We can't remove those plugins, we can only deprecate them. Any new code is net positive.

I see the point, and I can feel that pain first-hand from the tedium required to implement this plugin. But I wonder if the objective of minimizing maintenance burden would be better served by developing higher-level frameworks to develop plugins with less boilerplate, rather than to minimize the number of plugins.

Only oidcprovider.go, which is 15% of the server-side non-test code, has anything to do with OIDC. There's oauthBackend.verifyBoundClaims which is 10 line generic equality check on some attributes. The remaining approximately 670 lines of code does little more than move things from one data structure into another data structure with very little logic related to the essential complexity of this particular authentication method.

Combining OAuth with JWT would certainly amortize the boilerplate cost, but that doesn't seem like an especially elegant solution in the long run.

Combining OAuth with JWT would certainly amortize the boilerplate cost, but that doesn't seem like an especially elegant solution in the long run.

I don't really see what is inelegant. The vast majority of the JWT code is given to parsing and validating the JWT. This is still something that must be done whether the JWT comes from an OIDC workflow or not.

I have to be frank: I don't find your 2-sentence objections anything but frustrating. You asked for a design document and I gave you one complete with a proof of concept implementation. It's your turn to put in some effort towards a working solution.

Here are some questions you could try answering:

  • OIDC requires oidc_discovery_url and bound_issuer to be equal. How would that be handled in the JWT plugin? Would bound_issuer magically be set? What if the user provides a different value? Or would the user be required to explicitly set both, always equal?
  • To perform the OAuth flow, the Vault plugin must know its OAuth client ID. The sub claim must be equal to the OAuth client ID, so perhaps bound_audiences is synonymous with "OAuth client ID". However bound_audiences is an array which can have zero or more than one value: what is the client ID in this case?
  • The OAuth flow also requires a client secret. It would make sense to configure this alongside the client ID, but with the schema as it is, that means duplicating the client secret in each role. Is that an elegant schema?
  • When GitHub support is added, oidc_discovery_url and bound_issuer won't make any sense, though we still do require some kind of configuration to specify that GitHub (or whatever other providers may be added in the future) should be the issuer. How is that handled? A third, mutually exclusive /config parameter, like non_oidc_identity_provider? Or one of oidc_discovery_url or bound_issuer are overloaded?
  • After implementing these changes, do you think the user interface to the Vault administrator is a good one? Do the configuration endpoints and the parameters they accept match the mental model and expectations of the Vault administrator using them? Will the interdependencies, invariants, and semantics of the configuration items be easy to document and understand?

Hi Phil,

I'm sorry you've been frustrated, but it's hard to give anything other than vague answers to non-specific claims like inelegance, and I haven't known what your specific objections were. Now that you've listed some specific questions/concerns, I'm happy to discuss them.

OIDC requires oidc_discovery_url and bound_issuer to be equal. How would that be handled in the JWT plugin? Would bound_issuer magically be set? What if the user provides a different value? Or would the user be required to explicitly set both, always equal?

I think it depends on whether it makes sense for a single mount to be using various discovery URLs, thereby pushing configuration to roles (and deprecating the top-level value); or, whether it makes sense for a single mount to use a single discovery URL (so that different providers/discovery endpoints require different mounts). Either way, if a role is of type oidc instead of type jwt, it makes sense to simply ignore (with a warning) or reject any value for bound_issuer.

To perform the OAuth flow, the Vault plugin must know its OAuth client ID. The sub claim must be equal to the OAuth client ID, so perhaps bound_audiences is synonymous with "OAuth client ID". However bound_audiences is an array which can have zero or more than one value: what is the client ID in this case?

bound_audiences is for the aud claim, not the sub claim. It seems like matching the sub claim shouldn't be problematic.

The OAuth flow also requires a client secret. It would make sense to configure this alongside the client ID, but with the schema as it is, that means duplicating the client secret in each role. Is that an elegant schema?

I'm not sure offhand what you're doing in your proposed plugin, but...is there any reason not so simply follow that mold? Alternately, if the client ID is the same across many different potential client secrets, the client ID could live at top level plugin configuration. I think this really comes down to the same set of tradeoffs as oidc_discovery_url living at top-level of within each role.

When GitHub support is added, oidc_discovery_url and bound_issuer won't make any sense, though we still do require some kind of configuration to specify that GitHub (or whatever other providers may be added in the future) should be the issuer. How is that handled? A third, mutually exclusive /config parameter, like non_oidc_identity_provider? Or one of oidc_discovery_url or bound_issuer are overloaded?

I don't have an answer, but it's almost certainly not "overload a parameter with something that doesn't fit its description". I think if oauth becomes a role type, either a parameter that makes sense for that type could be used, or perhaps specific providers could have their own types, e.g. a role type of github that triggers certain checks.

After implementing these changes, do you think the user interface to the Vault administrator is a good one? Do the configuration endpoints and the parameters they accept match the mental model and expectations of the Vault administrator using them? Will the interdependencies, invariants, and semantics of the configuration items be easy to document and understand?

Sure.

I think it depends on whether it makes sense for a single mount to be using various discovery URLs, thereby pushing configuration to roles (and deprecating the top-level value); or, whether it makes sense for a single mount to use a single discovery URL (so that different providers/discovery endpoints require different mounts). Either way, if a role is of type oidc instead of type jwt, it makes sense to simply ignore (with a warning) or reject any value for bound_issuer.

I don't see any reason to abandon the 1 issuer per mount model. I expect most users will have 1 issuer. It doesn't make sense to repeat the same issuer configuration for each role.

I do want to point out that if bound_issuer is ignored or rejected when doing OIDC, this reduces the number of common parameters in the /config endpoint to zero.

bound_audiences is for the aud claim, not the sub claim. It seems like matching the sub claim shouldn't be problematic.

Sorry, typo. That should have been aud.

I'm not sure offhand what you're doing in your proposed plugin, but...is there any reason not so simply follow that mold?

Please do me the favor of reading the documentation and/or code before you ask that question.

I don't have an answer, but it's almost certainly not "overload a parameter with something that doesn't fit its description". I think if oauth becomes a role type, either a parameter that makes sense for that type could be used, or perhaps specific providers could have their own types, e.g. a role type of github that triggers certain checks.

I'm afraid I have to press for a specific answer. I'm aware overloading parameters is ugly: that's why I didn't do it that way in my implementation. I'm happy to entertain alternatives, but what you propose isn't specific enough to support a productive discussion. I've invested a lot of time to propose something complete and functional. I'd appreciate a commensurate effort on your end.

I do want to point out that if bound_issuer is ignored or rejected when doing OIDC, this reduces the number of common parameters in the /config endpoint to zero.

Sure, but there are (AFAICT) still a number of common parameters within role definitions.

Sorry, typo. That should have been aud.

OK, in that case we just require it to be a single audience and the same as the client ID; or, if the client doesn't supply it, simply set it to be equivalent.

I'm afraid I have to press for a specific answer. I'm aware overloading parameters is ugly: that's why I didn't do it that way in my implementation. I'm happy to entertain alternatives, but what you propose isn't specific enough to support a productive discussion. I've invested a lot of time to propose something complete and functional. I'd appreciate a commensurate effort on your end.

I've been trying to get answers to you to keep you unblocked. Usually answers to such questions are proposed by the person performing the implementation, and then evaluated in discussion. If you want me to generate the answers for you, it'll have to wait until such a time as I can do a deep dive into OAuth, OIDC, and your plugin. This won't be possible for at least a week and a half, and probably longer.

Sounds good, I can empathize with being busy. I'll check back in two weeks.

@jefferai any update?

Hi,

I think the main open question on our end is what your plan for the backend is -- do you want to upstream it? If so we can work on getting that going.

Yes, that would be great.

One open question I have is if it's possible to replace vault-login-oauth with vault login -method oauth or something of that sort. As far as I could tell there didn't seem to be any way to extend the Vault CLI in that way with a plugin, but also it looks like all the new auth methods are external plugins. It would be pretty cool if we could somehow roll that functionality into vault, or otherwise minimize the friction in getting that piece.

BTW, will you be at the DevOpsDays conference in Detroit on Wednesday? I live in the area, might see you there.

It's possible if we pull it in, yes.

A couple of high level notes:

  • The way this process goes is we create a repo on the HC org; you PR into it; we review and merge; we pull into Vault.

  • The license will need to be MPL2. That's our standard license, and our counsel won't allow exceptions :-)

  • I'll have Chris set up the repo later this week; he's managing a release in the next day or two so it'll be after that

  • As you can imagine, HashiConf coming up means we have our hands extremely full. I don't think this will end up being fully reviewed/merged until afterwards. Just want to set expectations.

Hi Phil,

The Vault team had a sync-up today, and I also synced up with Armon today. We've decided that the server side of the plugin is too similar to the JWT plugin to support it as a separate plugin, and would need to be merged.

The best way to see the similarities is when looking at role options, which arguably are the meat of the server side of both plugins (the plugin config options are more orthogonal but we don't see that as problematic).

Both plugins support ttl, max_ttl, policies, num_uses, and user_claim. The JWT plugin doesn't support bound_claims for arbitrary binding but that was already on our roadmap for support, and we would want to enhance it to support email_claim and given_name_claim. Meanwhile, the OIDC plugin doesn't support period, bound_subject, bound_cidrs, bound_audiences, or groups_claim, but we would want those to be supported on the OIDC side. Put the role params from each in the places we'd want them for each plugin and you have exactly the same set of role parameters. Since this controls validation, it also means that validation looks the same. Overall this reduces the difference between the server sides of the plugins to a fairly small set of configuration options, differing tests, and some other minor things.

The CLI support is obviously all-new, so there is no duplication to speak of, but it'd be as valid for a merged JWT/OIDC plugin as for a standalone OIDC plugin.

There is a pretty decent analogue of this as precedent: a while back we were asked to create a SPIFFE secrets backend, but given that SPIFFE is a specification dictating specific usage of the broader X.509 standard, it would have created a lot of code duplication to support both this and the existing PKI backend, so instead we enhanced PKI to make it possible to create certs with the OID key usages, OID SANs, and other things that SPIFFE mandates but that are also useful to other users that simply want to add these custom values to their X.509 certificates.

Please let us know your thoughts. We're hoping you'll still PR it because we'd love to have the functionality, and even if you're happy with that approach but don't want to spend the effort yourself, PRing with an appropriate license would let us be able to perform the merge for you.

Best,
Jeff

The best way to see the similarities is when looking at role options, which arguably are the meat of the server side of both plugins

I don't know why this is the best way when there's actual code we could be looking at, instead.

And really, the role options are "the meat" of the plugins?! Besides bound_claims, the role options have nothing to do with OAuth, or anything specific to this plugin really. They configure the parameters of the returned Vault token, something equally applicable to _any_ auth plugin, even though most auth plugins don't offer this degree of configuration.

If this is the meat of the plugin, it's because the plugin API is unnecessarily low level. If you want to manage maintenance overhead, _that_ is the problem you should be fixing. I'd love to help.

Meanwhile, the OIDC plugin doesn't support period, bound_subject, bound_cidrs, bound_audiences, or groups_claim

I guess you mean the OAuth plugin. I've said this before but it also supports GitHub. It's not something I need, but I thought it was something a lot of other people would want so I thought I'd give it a shot because it's not hard. All you have to do to see it completed is express some interest. The existing GitHub plugin (by your own documentation) has some security limitations. It's also tedious to use, requiring multiple manual steps. The Okta plugin also has some limitations. This is an opportunity to fix that. This is all explained in detail in the issue description. I think you should really consider that before dismissing this _OAuth_ plugin. I don't think you are proposing merging OAuth with the JWT plugin (you wrote: "Straight-up non-JWT OAuth is actually where I see more dissimilarity on the server side"), but somehow you seem to keep missing this point: it's an _OAuth_ plugin, not an _OIDC_ plugin.

bound_subject is supported through the more general bound_claims mechanism.

bound_cidrs isn't an OAuth thing: it's a Vault thing, in the same class as the other token options. A lot of plugins don't support it, even though they could and probably should. The proper way to address this is with a higher-level plugin API. Again, would love to help.

If you think bound_audiences support is missing, that only confirms my suspicions that you haven't read the code, don't understand OAuth, or both. There is only one audience that makes sense: the client ID. And I hope that is supported, because failing to validate that the audience claim is equal to the client ID would be an epic security error. But since this is an OAuth plugin, not an OIDC plugin, and OAuth doesn't have an audience claim, I'm not really sure why that option should exist here anyway.

groups_claim is something we could discuss: there's a reason I didn't include it.

I've said this before, but I've tried doing it your way, extending the JWT plugin. It seemed like the easy and right thing to do at first, but when I got in to it, it wasn't so easy. Just getting the configuration to make sense was hard. I've asked you for details on how you think it should be done, but I've yet to see a complete answer. The devil is in the details. You focus a lot on duplicated functionality, but I don't think you've considered the complexity of all the conditional branches that will have to be added to accommodate "JWT mode", versus "OIDC mode". Also consider the configuration complexity exposed to the administrator and the impact that will have on the usability of the product. I challenge you to try writing the documentation to explain it. I tried and it was confusing so I gave up. Finally consider the security risk of an implementation or misconfiguration error. A high cyclomatic complexity and a configuration schema that's hard to understand increases that risk.

but somehow you seem to keep missing this point: it's an OAuth plugin, not an OIDC plugin.

It's less that I'm misisng the point and more that I keep forgetting it, since what's in there now is support for OIDC. If you think that the OAuth aspect of it, e.g. for GitHub support, is more significantly different, it would be helpful to get an understanding of what that looks like. So far we've looked at the code that's there, but it's hard to know what you are intending w.r.t. OAuth support since it's not in the code, and thus it's hard to know how well it does or doesn't fit into any sort of unified plugin.

bound_subject is supported through the more general bound_claims mechanism.

Sure, but putting JSON objects into Vault through the CLI is quite difficult for users, and most users use the CLI. So for extremely common options, we'd want to have them as specific parameters that are easier to configure.

If you think bound_audiences support is missing

It shouldn't have been on the list, I just didn't discriminate it from the rest when composing it.

groups_claim is something we could discuss: there's a reason I didn't include it.

It seems very relevant to this discussion, it'd be good to know why you didn't include it.

@jefferai @bitglue Do we know the state of this issue? The OIDC authentication method will benefit a lot of people in the community.

I can't comment on this particular PR but I can tell you that OIDC support is on the roadmap.

Funny, you had a lot of comments (mostly negative) a month ago. Or did you not actually read the PR? :thinking:

@audip status of the PR is "Functional, but unable to merge because @jefferai rejected it without reading it". Same status it's been in for over 2 months.

Closing as OIDC will be supported in the next major release.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

trodemaster picture trodemaster  路  3Comments

andris9 picture andris9  路  3Comments

adamroddick picture adamroddick  路  3Comments

gtmtech picture gtmtech  路  3Comments

dwdraju picture dwdraju  路  3Comments