Review Request 36958: Fixed a bug in authentication validation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36958/ --- Review request for mesos and Vinod Kone. Repository: mesos Description --- When --flags.authenticate_frameworks is false, we still should be checking that the authenticated principal matches the framework's principal. Diffs - src/master/master.cpp 8162b2b796c9712a7b2d544118fe8de2646a8d32 Diff: https://reviews.apache.org/r/36958/diff/ Testing --- make check Thanks, Ben Mahler
Re: Review Request 36958: Fixed a bug in authentication validation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36958/#review93679 --- Ship it! src/tests/authentication_tests.cpp (line 199) https://reviews.apache.org/r/36958/#comment148095 s/,/ than Credential::principal/ ? s/when/even when/ ? do we already have a test for when authentication is required? if not, would be nice to have one. maybe parameterize this test? - Vinod Kone On July 30, 2015, 11:54 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36958/ --- (Updated July 30, 2015, 11:54 p.m.) Review request for mesos and Vinod Kone. Repository: mesos Description --- When --flags.authenticate_frameworks is false, we still should be checking that the authenticated principal matches the framework's principal. Diffs - src/master/master.cpp ce27dbe58d0afc2363991695e7af212616e1f09a src/tests/authentication_tests.cpp 5126ed4acbdf4c8508133bed35f98b0da3cca11e Diff: https://reviews.apache.org/r/36958/diff/ Testing --- make check Thanks, Ben Mahler
Re: Review Request 36958: Fixed a bug in authentication validation.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36958/#review93658
---
Mind adding a test for this if one doesn't exist already? Should be
straightforward.
src/master/master.cpp (lines 1565 - 1573)
https://reviews.apache.org/r/36958/#comment148069
why the nested if loop?
```
if (frameworkInfo.has_principal()
authenticated.contains(from)
fraemworkInfo.principal() != authenticated[from]) {
}
```
src/master/master.cpp (line 1567)
https://reviews.apache.org/r/36958/#comment148070
not yours, but
s/framweworks/frameworks/
- Vinod Kone
On July 30, 2015, 10:16 p.m., Ben Mahler wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36958/
---
(Updated July 30, 2015, 10:16 p.m.)
Review request for mesos and Vinod Kone.
Repository: mesos
Description
---
When --flags.authenticate_frameworks is false, we still should be
checking that the authenticated principal matches the framework's
principal.
Diffs
-
src/master/master.cpp 8162b2b796c9712a7b2d544118fe8de2646a8d32
Diff: https://reviews.apache.org/r/36958/diff/
Testing
---
make check
Thanks,
Ben Mahler
Re: Review Request 36958: Fixed a bug in authentication validation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36958/#review93668 --- Bad patch! Reviews applied: [36927] Failed command: ./support/apply-review.sh -n -r 36927 Error: 2015-07-30 23:07:34 URL:https://reviews.apache.org/r/36927/diff/raw/ [19346/19346] - 36927.patch [1] error: patch failed: src/master/master.hpp:575 error: src/master/master.hpp: patch does not apply error: patch failed: src/master/master.cpp:1524 error: src/master/master.cpp: patch does not apply Failed to apply patch - Mesos ReviewBot On July 30, 2015, 10:16 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36958/ --- (Updated July 30, 2015, 10:16 p.m.) Review request for mesos and Vinod Kone. Repository: mesos Description --- When --flags.authenticate_frameworks is false, we still should be checking that the authenticated principal matches the framework's principal. Diffs - src/master/master.cpp 8162b2b796c9712a7b2d544118fe8de2646a8d32 Diff: https://reviews.apache.org/r/36958/diff/ Testing --- make check Thanks, Ben Mahler
Re: Review Request 36958: Fixed a bug in authentication validation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36958/#review93687 --- Bad patch! Reviews applied: [36927] Failed command: ./support/apply-review.sh -n -r 36927 Error: 2015-07-31 00:49:14 URL:https://reviews.apache.org/r/36927/diff/raw/ [19346/19346] - 36927.patch [1] error: patch failed: src/master/master.hpp:575 error: src/master/master.hpp: patch does not apply error: patch failed: src/master/master.cpp:1524 error: src/master/master.cpp: patch does not apply Failed to apply patch - Mesos ReviewBot On July 30, 2015, 11:54 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36958/ --- (Updated July 30, 2015, 11:54 p.m.) Review request for mesos and Vinod Kone. Repository: mesos Description --- When --flags.authenticate_frameworks is false, we still should be checking that the authenticated principal matches the framework's principal. Diffs - src/master/master.cpp ce27dbe58d0afc2363991695e7af212616e1f09a src/tests/authentication_tests.cpp 5126ed4acbdf4c8508133bed35f98b0da3cca11e Diff: https://reviews.apache.org/r/36958/diff/ Testing --- make check Thanks, Ben Mahler
Re: Review Request 36958: Fixed a bug in authentication validation.
On July 31, 2015, 12:05 a.m., Vinod Kone wrote: src/tests/authentication_tests.cpp, line 199 https://reviews.apache.org/r/36958/diff/2/?file=1025323#file1025323line199 s/,/ than Credential::principal/ ? s/when/even when/ ? do we already have a test for when authentication is required? if not, would be nice to have one. maybe parameterize this test? Yeah, we already have a test, but not one for when authentication is not required. Looks like many of the AuthenticationTests can be parameterized, likely also need AuthenticationRequiredTest and AuthenticationNotRequiredTest to test unauthenticated frameworks. Will punt for now. Seems like we should change the phrasing here to say authentication required / not required rather than enabled / disabled? - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36958/#review93679 --- On July 30, 2015, 11:54 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36958/ --- (Updated July 30, 2015, 11:54 p.m.) Review request for mesos and Vinod Kone. Repository: mesos Description --- When --flags.authenticate_frameworks is false, we still should be checking that the authenticated principal matches the framework's principal. Diffs - src/master/master.cpp ce27dbe58d0afc2363991695e7af212616e1f09a src/tests/authentication_tests.cpp 5126ed4acbdf4c8508133bed35f98b0da3cca11e Diff: https://reviews.apache.org/r/36958/diff/ Testing --- make check Thanks, Ben Mahler
