Add roles CLI

[leovalais]

closes #8744

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 2.77778% with 105 lines in your changes missing coverage. Please review.

Project coverage is 38.54%. Comparing base (a1345d2) to head (5e12ec7).
Report is 11 commits behind head on dev.

Files with missing lines	Patch %	Lines
editoast/src/client/roles.rs	0.00%	95 Missing ⚠️
editoast/editoast_authz/src/authorizer.rs	0.00%	5 Missing ⚠️
editoast/editoast_authz/src/builtin_role.rs	16.66%	5 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff              @@
##                dev    #9211      +/-   ##
============================================
- Coverage     38.58%   38.54%   -0.05%     
  Complexity     2242     2242              
============================================
  Files          1285     1286       +1     
  Lines         97808    97911     +103     
  Branches       3254     3254              
============================================
- Hits          37737    37735       -2     
- Misses        58142    58247     +105     
  Partials       1929     1929              

Flag	Coverage Δ
core	74.78% <ø> (ø)
editoast	71.99% <2.77%> (-0.28%)	⬇️
front	10.22% <ø> (ø)
gateway	2.22% <ø> (ø)
osrdyne	2.46% <ø> (ø)
railjson_generator	87.49% <ø> (ø)
tests	86.71% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[flomonster]

LGTM.

I came across two irritating little things while testing the CLI.

[woshilapin]

Tested and might be missing some information on the CLI. Here is an example (I removed all tracing logs).

❯ export RUST_LOG=none 
❯ editoast roles list 1
morane#1 (Bob Morane) has no roles assigned

❯ editoast roles add 1 SuperUser StdcmAdmin

❯ editoast roles list 1
Superuser
StdcmAdmin

❯ editoast roles remove 1 SuperUser StdcmAdmin

A few things to note that could be improved.

• When adding roles, there is no message confirming everything went fine.
• When removing roles, there is no message confirming everything went fine.
• When adding or removing roles of a user that doesn't exist, there is also no message.
• When listing roles and the user has no roles, the user and its ID are clearly identified... but not when the user has roles, only the roles are displayed: maybe having also the user printed would be nice

Another problem I see is that I would expect add to append roles, but it actually overwrites the existing list. See for example.

❯ editoast roles add 1 SuperUser StdcmAdmin
❯ editoast roles list 1
Superuser
StdcmAdmin
❯ editoast roles add 1 OpsWrite
❯ editoast roles list 1
OpsWrite

I would expect the last command to output 3 values.

Would it make sense to add some tests about it?

[leovalais]

As discussed:

Another problem I see is that I would expect add to append roles, but it actually overwrites the existing list.

Can't reproduce.

A few things to note that could be improved.
When adding roles, there is no message confirming everything went fine.
When removing roles, there is no message confirming everything went fine.
When adding or removing roles of a user that doesn't exist, there is also no message.
When listing roles and the user has no roles, the user and its ID are clearly identified... but not when the user has roles, only the roles are displayed: maybe having also the user printed would be nice

My logic for choosing between logging and println! was that logging is only meant to be informative and provide context while println! is an output that can be used in a script (for piping into another process for ex.). This is debatable and can be changed ofc.

@woshilapin Wdyt?

[woshilapin]

As discussed:

Another problem I see is that I would expect add to append roles, but it actually overwrites the existing list.

Can't reproduce.

Can't reproduce either, I might have messed up somewhere, sorry.

My logic for choosing between logging and println! was that logging is only meant to be informative and provide context while println! is an output that can be used in a script (for piping into another process for ex.). This is debatable and can be changed ofc.

@woshilapin Wdyt?

I agree, that was a point of view I didn't have and a totally valid one. Fine with me.

[woshilapin]

Thank you. Let's merge that 😄

[flomonster]

Thanks!