Page MenuHomePhabricator

[services] Tunnelbroker - Add skip authentication config parameter
ClosedPublic

Authored by max on Jan 12 2023, 8:03 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, May 13, 6:44 PM
Unknown Object (File)
Thu, May 9, 6:27 PM
Unknown Object (File)
Sun, Apr 21, 9:18 PM
Unknown Object (File)
Sun, Apr 21, 9:18 PM
Unknown Object (File)
Sun, Apr 21, 9:18 PM
Unknown Object (File)
Sun, Apr 21, 9:18 PM
Unknown Object (File)
Sun, Apr 21, 9:16 PM
Unknown Object (File)
Mar 12 2024, 11:17 PM
Subscribers

Details

Summary

This diff introduces adding the sessions.skip_authentication config flag. It will be used in D6253 to skip the authentication mechanism.
As we don't have a function to get boolean flags from the config file (we have string parameters), the internal isParameterSet function was created to get the boolean value of the flag.

Linear task: ENG-2642

Test Plan

Adding the skip_authentication flag to the [sessions] section of the config file results in a true return from the isParameterSet('sessions.skip_authentication') function call.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

max held this revision as a draft.
max edited the test plan for this revision. (Show Details)
max added a reviewer: bartek. max added 1 blocking reviewer(s): jon.
max published this revision for review.Jan 12 2023, 8:21 AM
jon added inline comments.
services/tunnelbroker/src/libcpp/src/Tools/ConfigManager.cpp
150–155 ↗(On Diff #20872)

Feel like this should be equivalent.

I dislike if (...) { return true; } else { return false; } usage.

This revision now requires changes to proceed.Jan 12 2023, 11:28 AM
services/tunnelbroker/src/libcpp/src/Tools/ConfigManager.cpp
150–155 ↗(On Diff #20872)

I cannot find any docs about return type of count(), but if it's boolean, then you can simply
return this->variablesMap.count(param);

If it is integer-like, then more idiomatic would be

return this->variablesMap.count(param) != 0;

instead of (bool) casting,
but up to you

Return in a more elegant way.

max added inline comments.
services/tunnelbroker/src/libcpp/src/Tools/ConfigManager.cpp
150–155 ↗(On Diff #20872)

count() actually returns a count of occurrences but 0,1 casting to bool.
We can use this elegant way: return this->variablesMap.count(param) != 0; as @bartek suggested, eliminate a possible bug where multiple occurrences of the flag can happen.
Thanks @jon, @bartek !

This revision is now accepted and ready to land.Jan 25 2023, 9:44 AM