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.

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

Multiuser and OpenId #447

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
Draft

Multiuser and OpenId #447

wants to merge 11 commits into from

Conversation

lelemm
Copy link

@lelemm lelemm commented Sep 4, 2024

No description provided.

Copy link

netlify bot commented Sep 4, 2024

Deploy Preview for actualbudget-website ready!

Name Link
🔨 Latest commit f979124
🔍 Latest deploy log https://app.netlify.com/sites/actualbudget-website/deploys/66d8b23e3cc4c4000839e4ae
😎 Deploy Preview https://deploy-preview-447.www.actualbudget.org
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

docs/advanced/oauth-auth.md Fixed Show fixed Hide fixed
docs/advanced/oauth-auth.md Fixed Show fixed Hide fixed
docs/advanced/oauth-auth.md Fixed Show fixed Hide fixed
docs/advanced/oauth-auth.md Fixed Show fixed Hide fixed

This comment has been minimized.

This comment has been minimized.

docs/advanced/oauth-auth.md Fixed Show fixed Hide fixed

This comment has been minimized.

Copy link
Member

@RubenOlsen RubenOlsen left a comment

Choose a reason for hiding this comment

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

I hope I have not been to crass in my comments and suggestions.

Also please do not use the backtick when referring to elements in the UI, use italics or bold to reference the elements and bold or italics to reference the content of the elements. Italics are preferred over bold to reference elements.


## Setup

This feature needs to be enabled on the server, it is not configured to work out of the box. In the Actual config, set the value `multiuser` or env `ACTUAL_MULTIUSER` to `"true"`. This will enable multiuser support.
Copy link
Member

Choose a reason for hiding this comment

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

The left menu item in the program is called "Actual Setup" and not "Actucal config". I think the documentation needs to reflect this.

I would also suggest that you show a screenshot.

This feature needs to be enabled on the server, it is not configured to work out of the box. In the Actual config, set the value `multiuser` or env `ACTUAL_MULTIUSER` to `"true"`. This will enable multiuser support.

:::warning
The first user to login, after enabling multiuser (and OpenID Provider), will be considered the `master` user. Master users **cannot** be deleted nor change the role from Admin to Basic. But you can change the username manually.
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
The first user to login, after enabling multiuser (and OpenID Provider), will be considered the `master` user. Master users **cannot** be deleted nor change the role from Admin to Basic. But you can change the username manually.
The first user to log in after enabling multiuser (and OpenID Provider) will be considered the `master` user. Master users **cannot** be deleted or their role changed from Admin to Basic, but their usernames can be changed manually.


![](/static/img/multiuser/user-directory-overview.png)

Users can be `Basic` or `Admin`.
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
Users can be `Basic` or `Admin`.
There are two user roles _Basic_ or _Admin_.


Users can be `Basic` or `Admin`.

- Basic:
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
- Basic:
- The Basic role:

Users can be `Basic` or `Admin`.

- Basic:
Users with the Basic role can create new budgets and be invited to collaborate on budgets created by others.
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
Users with the Basic role can create new budgets and be invited to collaborate on budgets created by others.
Users with the Basic role can create new budgets and collaborate on budgets made by others.

Shorter sentences are easier to read than longer ones.


#### After setup:

You will be redirected to login page:
Copy link
Member

Choose a reason for hiding this comment

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

This is an incomplete sentence. My suggestion:

Suggested change
You will be redirected to login page:
When setup is done, you will be redirected to the _login_ page:

![](/static/img/oauth/first-login.png)

### Next Step
Once OpenID provider is setup. Setup [Multiuser](multi-user)
Copy link
Member

Choose a reason for hiding this comment

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

Incomplete sentences. My suggestion:

Suggested change
Once OpenID provider is setup. Setup [Multiuser](multi-user)
Once you have configured your OpenID, you can continue to enable [Multiuser](multi-user) login.



:::tip
Configuring OpenID provider from options or bootstrap can be only done if your provider supports discovery, otherwise, use [file configuration](oauth-auth#config-using-configuration-file)
Copy link
Member

Choose a reason for hiding this comment

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

A lot is going on there at the same time. Alo be only should be only be. Bootstrap is, as stated above, a bit iffy to use.

I suggest the following:

Suggested change
Configuring OpenID provider from options or bootstrap can be only done if your provider supports discovery, otherwise, use [file configuration](oauth-auth#config-using-configuration-file)
Configuring the OpenID provider from options or during the initial setup for Actual can only be done if your provider supports discovery; otherwise, use [file configuration](oauth-auth#config-using-configuration-file)

Server Version 24.10.0 or higher are required for this feature.
:::

This feature enables multiple users to login into Actual. For now, you need to enable [OpenId Provider auth](oauth-auth). The usernames will be fetched from the provider.
Copy link
Member

Choose a reason for hiding this comment

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

You need to clearly state that to before you can configure Multi User Support, you need to enable OpenID.

@@ -0,0 +1,50 @@
# Multi user support
Copy link
Member

Choose a reason for hiding this comment

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

Usually, each word in the headings is in upper case. If you prefer not to have this, you must be consistent across the document.

Multi User is spelled with a hypen.

Also - maybe change the title to:

Suggested change
# Multi user support
# Managing Multi-User Support

@lelemm
Copy link
Author

lelemm commented Nov 27, 2024

@RubenOlsen thanks for your dedication on this. Took this long for me to notice that you reviewed this.
There was multiple changes on how multiuser and openid was implemented since this PR. I'm waiting for the frontend PR be approved, than I will come back to this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants