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

Use ‘has’ operator with claim subpath in PPL #2851

Closed
robertgates55 opened this issue Dec 25, 2021 · 5 comments
Closed

Use ‘has’ operator with claim subpath in PPL #2851

robertgates55 opened this issue Dec 25, 2021 · 5 comments

Comments

@robertgates55
Copy link

robertgates55 commented Dec 25, 2021

We're using dex as our IDP, which pulls user groups from google and passes these back as a user claim - the groups are not direct pomerium groups.

Is it possible to use the claim field with a subpath and the 'has' operator? I'd like to do something like this:

allow:
  and:
  - domain:
      is: du.co
  - claim/groups:
      has: mygroup

I've seen that I can combine with existing annotations, eg:

kind: Ingress
metadata:
  annotations:
    ingress.pomerium.io/allowed_idp_claims: |
      groups:
      - platform@du.co
    ingress.pomerium.io/policy: |
      allow:
        and:
        - domain:
            is: du.co

But this seems to apply the allowed_idp_claims policy as an OR with the policy... which isn't what I'm after.

Any options here?

@calebdoxsey
Copy link
Contributor

@robertgates55 IDP claims are always a map of string keys to lists of values:

{
  "family_name": ["Doe"],
  "given_name": ["John"]
}

So the claim criterion implicitly uses the has operator. You should be able to create a policy by removing the has operator and just using the value directly:

allow:
  and:
  - domain:
      is: du.co
  - claim/groups: mygroup

The criterion should return true if any of the values in the list match the value. (in this case if any of the groups are mygroup)

Have you tried this?

@calebdoxsey calebdoxsey self-assigned this Dec 29, 2021
@robertgates55
Copy link
Author

Aha!

I had not, but that works perfectly - thanks.

Fwiw - I'd read the docs, and "if a token claim matches the supplied value exactly" had led me to think I'd need to string-match the whole array... I now see that that's not what it says. Might be worth clarifying though for future people, or maybe supporting the has matcher as well for consistency and policy readability?

@travisgroth travisgroth added the docs Docs update required label Jan 3, 2022
@travisgroth
Copy link
Contributor

Hi @robertgates55 do you have a suggestion on where/how we could clarify? We'd be glad for some input there.

@desimone desimone added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jan 7, 2022
@nhayfield
Copy link
Contributor

@travisgroth I think he is just saying that we should support has/is as well as just : for consistency

@nhayfield nhayfield reopened this Feb 2, 2022
@alexfornuto alexfornuto added NeedsDiscussion and removed docs Docs update required WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Feb 2, 2022
@alexfornuto
Copy link
Contributor

Closing per @desimone

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

No branches or pull requests

6 participants