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
Feature/add consultancy account #877
Conversation
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good start!
I found a mix-up in how it's supposed to work, though.
Can you edit the PR description (only relevant sections), and then also reference discussion #868
Once we have this working well, I think I'd like to implement my idea of the required user role, as well (see discussion: "Addendum: also require a user role"). Better to do that right away, because otherwise the access might apply to too many people.
…and fix 3.8 support Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
…to read Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
…r role Signed-off-by: GustaafL <guus@seita.nl>
I've added the customer-manager user role as a requirement for accessing consultant client accounts by consultants. |
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good iteration! Only smaller comments at this point.
Maybe we should add a small sentence/paragraph to https://flexmeasures.readthedocs.io/en/latest/concepts/security_auth.html#authorization, to explain this concept.
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very close to done.
I'd like to add the consultant's account ID to the client account when I'm creating it on the CLI (we discussed that once in person).
That would also make it easy enough for me to test this PR locally.
…client accounts Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
I've added the option to add a consultancy-account-id in the CLI and a test that this works as expected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested it (via the UI) and it works.
I feel that one part is too confusing for later, and needs more clarity. Then we can merge!
Signed-off-by: GustaafL <guus@seita.nl>
Signed-off-by: GustaafL <guus@seita.nl>
I used the following psql statements to get a short demo going. Perhaps the first statement is suited to already be included in the database migration.
UPDATE: we found out that we haven't created new user roles before within database migrations, so my suggestion is maybe not the way to go. What we had done before was to add new user roles to a list in |
Signed-off-by: GustaafL <guus@seita.nl>
So we should add two new roles there in our current PR? Thanks. |
Signed-off-by: GustaafL <guus@seita.nl>
Done |
… Sensor (#890) * fix: hierarchical read rights from Account to Asset and from Asset to Sensor Signed-off-by: F.N. Claessen <felix@seita.nl> * style: flake8 Signed-off-by: F.N. Claessen <felix@seita.nl> * feat(test): add test for getting single asset in consultantclient account Signed-off-by: GustaafL <guus@seita.nl> * update docstrings to match new reality Signed-off-by: Nicolas Höning <nicolas@seita.nl> --------- Signed-off-by: F.N. Claessen <felix@seita.nl> Signed-off-by: GustaafL <guus@seita.nl> Signed-off-by: Nicolas Höning <nicolas@seita.nl> Co-authored-by: GustaafL <guus@seita.nl> Co-authored-by: Nicolas Höning <nicolas@seita.nl>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
Description
Add a Consultant account that has read access to accounts of clients.
Use case: Our customer is the supplier to a bunch of other (client) accounts on the server. They should have read-only access to some accounts. At least this is the first step. So they can oversee what happens, for instance for servicing questions and problems.
Look & Feel
The consultant will be able to see the accounts in their overview page and the assets they have access to. The consultant client will be able to see which account has read access to their account.
How to test
Tests are added in flexmeasures/api/v3_0/tests/test_assets_api.py
To test in the UI create a consultant account and a consultant client account in the CLI. Then go to the account pages and see the accounts represented.
Further Improvements
Potential improvements to be done in the same PR or follow up Issues/Discussions/PRs.
Related Items
The technical discussion can be found here: #868
The issue that this closes can be found here: #203