Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Authorization documentation #300

Merged
merged 3 commits into from
Nov 18, 2021
Merged

Authorization documentation #300

merged 3 commits into from
Nov 18, 2021

Conversation

datamel
Copy link
Contributor

@datamel datamel commented Sep 28, 2021 •
edited
Loading

Accompanies cylc/cylc-uiserver#204.

Partially addresses Document Configurable Authorization #271.

Documents authorization and adds a new section, Sharing Workflows to the user-guide.

@datamel datamel added the content Addition or modification of documentation label Sep 28, 2021
@datamel datamel added this to the 8.0.0 milestone Sep 28, 2021
@datamel datamel self-assigned this Sep 28, 2021
@datamel datamel mentioned this pull request Sep 28, 2021
Merged
5 tasks
oliver-sanders
oliver-sanders reviewed Oct 5, 2021
View reviewed changes
src/user-guide/sharing-workflows/sharing-workflows.rst Outdated Show resolved Hide resolved
src/user-guide/sharing-workflows/sharing-workflows.rst Outdated Show resolved Hide resolved
src/user-guide/sharing-workflows/sharing-workflows.rst Outdated Show resolved Hide resolved
src/user-guide/sharing-workflows/sharing-workflows.rst Outdated Show resolved Hide resolved
src/user-guide/sharing-workflows/sharing-workflows.rst Outdated Show resolved Hide resolved
src/user-guide/sharing-workflows/sharing-workflows.rst Outdated Show resolved Hide resolved
src/user-guide/sharing-workflows/sharing-workflows.rst Outdated Show resolved Hide resolved
src/user-guide/sharing-workflows/sharing-workflows.rst Outdated Show resolved Hide resolved
src/user-guide/sharing-workflows/sharing-workflows.rst Outdated Show resolved Hide resolved
src/user-guide/sharing-workflows/sharing-workflows.rst Outdated Show resolved Hide resolved
hjoliver
hjoliver requested changes Oct 16, 2021
View reviewed changes
Copy link
Member

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a bunch of minor suggestions and questions.

src/user-guide/sharing-workflows/sharing-workflows.rst Outdated Show resolved Hide resolved
src/user-guide/sharing-workflows/sharing-workflows.rst Outdated Show resolved Hide resolved
src/user-guide/sharing-workflows/sharing-workflows.rst Outdated Show resolved Hide resolved
src/user-guide/sharing-workflows/index.rst Outdated Show resolved Hide resolved
src/user-guide/sharing-workflows/sharing-workflows.rst Outdated Show resolved Hide resolved
src/user-guide/sharing-workflows/sharing-workflows.rst Outdated Show resolved Hide resolved
src/user-guide/sharing-workflows/sharing-workflows.rst Outdated Show resolved Hide resolved
src/user-guide/sharing-workflows/sharing-workflows.rst Outdated Show resolved Hide resolved
src/user-guide/sharing-workflows/sharing-workflows.rst Outdated Show resolved Hide resolved
src/user-guide/sharing-workflows/sharing-workflows.rst Outdated Show resolved Hide resolved
oliver-sanders
oliver-sanders reviewed Oct 20, 2021
View reviewed changes
Copy link
Member

@oliver-sanders oliver-sanders left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, some minor comments.

CI is failing, perhaps try rebasing/pushing to trigger a build with up to date cylc-flow/cylc-uiserver.

src/user-guide/sharing-access-to-workflows/index.rst Outdated Show resolved Hide resolved
src/user-guide/sharing-access-to-workflows/sharing-access-to-workflows.rst Outdated Show resolved Hide resolved
src/user-guide/sharing-access-to-workflows/sharing-access-to-workflows.rst Outdated Show resolved Hide resolved
src/user-guide/sharing-access-to-workflows/sharing-access-to-workflows.rst Outdated Show resolved Hide resolved
src/user-guide/sharing-access-to-workflows/sharing-access-to-workflows.rst Outdated
Such behaviours cannot be controlled through technical measures so must be
enforced by site policy. By the same token, however, because Cylc UI Servers
(and Schedulers) run as the user, one user cannot compromise another user's
account using Cylc.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
account using Cylc.
UI Server.

Copy link
Member

@hjoliver hjoliver Oct 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sentence was suggested by me. I'm not sure I worded it very well, or that your suggestion here is correct either. Technically, granting another user access to my workflows could (or does) compromise my UIS and my account, but that's by design. What I meant to suggest was that granting another user access to my account cannot compromise any other users' account (because everything I granted access to runs as me).

src/user-guide/sharing-access-to-workflows/sharing-access-to-workflows.rst Outdated Show resolved Hide resolved
@oliver-sanders oliver-sanders mentioned this pull request Nov 8, 2021
Merged
@MetRonnie MetRonnie modified the milestones: 8.0.0, 8.0b3 Nov 12, 2021
@datamel
Add authorization docs
8f47e15
@datamel
Tidy
ffed206
@datamel
Add not for diabled buttons for unauthorised mutations
6051775 
update 7-8

Add link to authorisation from architecture

remove ping from READ
@datamel
Copy link
Contributor Author

datamel commented Nov 18, 2021

Rebased. There is still an unresolved comment relating to this sentence...
By the same token, however, because Cylc UI Servers (and Schedulers) run as the user, one user cannot compromise another user's account using Cylc.
Any strong thoughts @hjoliver?
I have attempted to rewrite with little success, how about something like...
By the same token, however, since these processes (Cylc UI Servers and Schedulers) run as the owner, granting a user access has no security implications for *their* account.

@hjoliver
Copy link
Member

Thanks @datamel I'll get this merged. Will reconsider the hard-to-word sentence as a follow-up.

@hjoliver hjoliver merged commit 7962b48 into cylc:master Nov 18, 2021
@hjoliver hjoliver mentioned this pull request Nov 18, 2021
Merged
@datamel datamel deleted the authorization-documentation branch October 6, 2022 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hjoliver hjoliver hjoliver approved these changes

@oliver-sanders oliver-sanders oliver-sanders approved these changes

@dwsutherland dwsutherland Awaiting requested review from dwsutherland

Assignees

@datamel datamel

Labels
content Addition or modification of documentation
Projects
None yet
Milestone
8.0b3
Development

Successfully merging this pull request may close these issues.
4 participants
@datamel @hjoliver @oliver-sanders @MetRonnie