Add "MFA Verified" check to workflowengine #36045
Conversation
michielbdejong
force-pushed
the
feat/mfaverified-check
branch
from
16fc4bb
to
d6c2541
Compare
PVince81
requested review from
nickvergessen,
blizzz,
ChristophWurst and
come-nc
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| $this->container->query(FileName::class), | ||
| $this->container->query(FileSize::class), | ||
| $this->container->query(FileSystemTags::class), | ||
| $this->container->query(MfaVerified::class), |
Check notice
Code scanning / Psalm
DeprecatedMethod
| class: 'OCA\\WorkflowEngine\\Check\\MfaVerified', | ||
| name: t('workflowengine', 'MFA Verified'), | ||
| operators: [ | ||
| { operator: 'is', name: t('workflowengine', 'is verified?') }, |
There was a problem hiding this comment.
and is not?
That would allow e.g. to use access control to block access to folders for people that did not 2fa
There was a problem hiding this comment.
Yes, this is a default boolean dropdown (@mrvahedi68 will provide some screenshots of it later today); the operatorof the check is always "is" and the value of the check can be "true" or "false".
So that way both "MFA verified is true" and "MFA verified is false" are available. Indeed, for blocking access to a folder you would create a rule to "block access" if "MFA verified is false".
There was a problem hiding this comment.
You can see here to screenshots of it. We have a dropdown with only one field 'is verified? and another textbox with a hint (true) that user can input anything. So We have a check at this function to validate user input. (you can see a related screenshot below). Based on this rule admin can restrict access to files by MFA verified is false or true.
There was a problem hiding this comment.
Ah okay. Because that is trouble some with translations and admins using different langauges, I would recomment to:
- Operation:
- is
- is not
- Value (hard coded to):
- verified
There was a problem hiding this comment.
When you use a dedicated component you can even remove the input field for the value.
There was a problem hiding this comment.
I changed the source and use the new approach you described above.
Now we have only one drop-down with two values and no other inputs.
You can find screenshots at the flowing.
@nickvergessen
|
Not sure why |
|
Ah, need to run |
|
The failing test seems unrelated. @mrvahedi68 can you sign off your commits? |
Failing lint and node are related. Fixable with: And then commit. If you fail to do that, that's no problem, we can easily take that over. |
michielbdejong
force-pushed
the
feat/mfaverified-check
branch
from
6a54f50
to
0f34247
Compare
Author: MohammadReza Vahedi (@mrvahedi68) Signed-off-by: Michiel de Jong <michiel@unhosted.org>
Signed-off-by: Michiel de Jong <michiel@unhosted.org>
Signed-off-by: Michiel de Jong <michiel@unhosted.org>
Signed-off-by: Michiel de Jong <michiel@unhosted.org>
michielbdejong
force-pushed
the
feat/mfaverified-check
branch
from
0f34247
to
6a98590
Compare
Signed-off-by: Michiel de Jong <michiel@unhosted.org>
michielbdejong
force-pushed
the
feat/mfaverified-check
branch
from
ca9f8b1
to
cd13b39
Compare
|
This PR seems to be using the wrong branch, will recreate it. |
This PR goes together with:
It allows the creation of work flows that depend on whether the logged-in user has passed MFA Verification for the current session or not.