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

Cannot see values in UI with nested ACL #1747

Closed
ashald opened this issue Feb 22, 2016 · 9 comments
Closed

Cannot see values in UI with nested ACL #1747

ashald opened this issue Feb 22, 2016 · 9 comments
Labels
thinking More time is needed to research by the Consul Contributors type/enhancement Proposed improvement or new feature

Comments

@ashald
Copy link
Contributor

ashald commented Feb 22, 2016

With Consul 0.6.3 and the latest UI bundle (due to #1071) let's assume following KV hierarchy:

/foo
     alpha
     beta
     gamma
/bar

With ACLs enabled and default policy set to deny if ACL for token set to:

key "bar/" {
  policy = "read"
}

then in UI you will see bar and all its content. But if ACL is set to

key "foo/alpha/" {
  policy = "read"
}

you won't be able to see foo in the UI. In order to get access to alpha you will need to manually put foo into your URL.

Expected behavior: with ACL as mentioned above you should be able to use UI to navigate up to the target dir.

@slackpad slackpad added type/enhancement Proposed improvement or new feature thinking More time is needed to research by the Consul Contributors labels Feb 22, 2016
@slackpad
Copy link
Contributor

Hi @ashald this is a function of the longest prefix match behavior of ACLs (see https://groups.google.com/d/msgid/consul-tool/e1e286f7-f091-420f-93ee-3939268a23e9%40googlegroups.com?utm_medium=email&utm_source=footer). I think we'd need a special-case "list" permission to change this behavior. Consul KV doesn't really have knowledge of folders, though, so we'd need to see how this would look.

@ashald
Copy link
Contributor Author

ashald commented Feb 22, 2016

But how does that work with

key "bar/" {
  policy = "read"
}

?

I assume when you hit on K/V button UI makes a call to get contents of /, root record, but still displays bar.

@slackpad
Copy link
Contributor

Yeah - it will list / and pass in the delimiter / so it will see bar/ and foo/ but there won't be an ACL that lets it see foo/ since the foo/alpha/ ACL is a more specific prefix, so that will be filtered out when looking at the top level.

@ashald
Copy link
Contributor Author

ashald commented Feb 22, 2016

Ah, I see - it will compare exact key names with ACLs, right?

It'd be so nice to have simple fnmatch-alike wildcards though...

@slackpad
Copy link
Contributor

Correct - it finds the longest prefix match against the ACLs and uses that. Wildcards are a good suggestion for this!

@ashald
Copy link
Contributor Author

ashald commented Feb 23, 2016

@slackpad can you please take a look at #1738 ? I believe it also might be an issue related to ACL resolution.

@intelradoux
Copy link

I'm also interested by this enhancement.

From the example

/foo
  alpha
  beta
  gamma
/bar

I was thinking of ACL like (default set to deny):

key "foo" {  policy = "read" }
key "foo/*" {  policy = "deny" }
key "foo/beta/" {  policy = "write" }

we can also think of rule like :

key "foo/**/password" {  policy = "deny" }  
key "bar/*/password" {  policy = "deny" }

"**" meaning any number of level
"*" meaning only one level.

in order to hide some stuff for specific people.

If I got some time I will look source to check if I can do that

@ghost
Copy link

ghost commented Mar 20, 2017

I agree that support for wildcards would be of great benefit to us. We have configuration data stored under different namespaces each ACL'ed off to the team that owns that namespace. Right now that involves explicitly denying every other namespace in every ACL. 40 teams = 1600 ACL rules and adding a rule to all 40 ACLs any time a new team is created :( . Being able to reduce that down to what @intelradoux suggested would be awesome.

@slackpack is there any update on possibly getting this support added?

@slackpad
Copy link
Contributor

slackpad commented May 9, 2017

Closing this against #3025 which captures the wildcard portion of this.

@slackpad slackpad closed this as completed May 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
thinking More time is needed to research by the Consul Contributors type/enhancement Proposed improvement or new feature
Projects
None yet
Development

No branches or pull requests

3 participants