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

Support for negated command alias in /etc/sudoers - fixes #262 #263

Conversation

@GeoffWilliams
Copy link
Contributor

GeoffWilliams commented Jul 10, 2015

Hi there,

This fixes the sudoers lens to support negating command alias groups like this:

 %sportshorseracingopssudoers ALL=(ALL) ALL, !DISALLOWED

Which currently fails if the command alias appears after a comma.

An additional unit test has been added to support this fix which is also documented at https://fedorahosted.org/augeas/ticket/346 (along with a patch which was a useful confirmation that the fix I'd come up with was correct)

How does this look? Would be great to get this fixed once and for all.

Thanks,
Geoff

@@ -326,3 +326,16 @@ test Sudoers.spec get "group+user somehost = ALL\n" =
{ "command" = "ALL" }
}
}

(* Test: Sudoers.spec
https://github.com/hercules-team/augeas/issues/262: Sudoers lens doesn't suppot `!` for command aliases *)

This comment has been minimized.

Copy link
@raphink

raphink Jul 13, 2015

Member

typo in comment

A reference to GH #263 might be enough.

@@ -88,7 +88,7 @@ let sep_dquote = Util.del_str "\""
(* Variable: sto_to_com_cmnd
sto_to_com_cmnd does not begin or end with a space *)
let sto_to_com_cmnd =
let alias = Rx.word - /(NO)?(PASSWD|EXEC|SETENV)/
let alias = /!?/ . Rx.word - /(NO)?(PASSWD|EXEC|SETENV)/

This comment has been minimized.

Copy link
@raphink

raphink Jul 13, 2015

Member

I'd rather use the negate and negate_node lenses, so it behaves like the rest of the lens:

let alias = del_negate . negate_node? . Rx.word - /(NO)?(PASSWD|EXEC|SETENV)/
… tree for negated commands and command alias; update testcase to reflect this and add a test for building a negated command for completeness
@GeoffWilliams

This comment has been minimized.

Copy link
Contributor Author

GeoffWilliams commented Jul 14, 2015

Hi Raphink,

Thanks for looking at this. I've updated the PR as you asked. To get the negate and negate_node lenses to work I had to re-arrange the line a little or I got an error about concatenating a lens with a regexp.

I also had to move some definitions to be earlier in the file so that I could reference them. I've updated the tests to reflect the { "negate" } node that now gets created and have also updated the definition of non_alias as this previously had a hardcoded regexp for a single ! character.

@raphink

This comment has been minimized.

Copy link
Member

raphink commented Aug 3, 2015

I understand the reorganization due to lenses needing other lenses. However, it means that some defaults definitions end up outside the defaults group, which will generate an ugly doc. Would it be possible to exchange the two groups altogether maybe?

@raphink

This comment has been minimized.

Copy link
Member

raphink commented Sep 15, 2015

Merged as 58a009f

@raphink raphink closed this Sep 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.