SSO support #2578

privacyguard · 2024-07-02T21:28:47Z

This PR is based on LemmyNet/lemmy#4238 and #2278 by @thepaperpilot.

We noticed that the original PR is outdated and has a lot of conflicts with the recent changes. We tried to keep the previous commits whenever possible (in lemmy-js-client and lemmy-ui).

How is works?

Admins can configure external OIDC providers from within the admin settings.
Once an OIDC provider is configured, users will be able to Sign In / Sign Up using external OIDC providers.
Available Configuration

The usual OIDC endpoints

auto_verify_email: When enabled, users signing up using OIDC won't need to go through email verification.
auto_approve_application: When enabled, users signing up using OIDC won't need manual approval even if applications are required.
account_linking_enabled: When enabled, users attempting with sign up with OIDC using an existing user email would link the OIDC account to the existing user.
Disclaimer
This is our first ever rust contribution.

Who we are? Why are we contributing to Lemmy?

Privacy Portal is an OIDC provider and an email aliasing service focused on privacy. We have decided to contribute to select open source projects that empower Free Speech online.
Our OIDC provider services are currently offered free of charge. In the future, we will have a generous free plan that will cover most deployments.
Using Privacy Portal as your OIDC provider offers your users great privacy benefits. User emails will automatically get replaced by single-purpose Privacy Aliases during sign up. Users will be able to enter any name (to be used as username). Users can benefit from email encryption and much more.

privacyguard · 2024-07-02T21:30:37Z

Please note that this PR depends on the latest lemmy-js-client (which doens't currently have a published version to use)

privacyguard · 2024-09-18T16:11:09Z

@dessalines
We've updated this PR with the SSO changes but it seems that the latest lemmy-js-client includes additional changes that currently would break the main banch.

The issues can be seen in the linting results.

Some of the changes are related to the following changes amongst others:
LemmyNet/lemmy#4841
LemmyNet/lemmy#5017

We will wait until these are addressed in main (with the latest lemmy-js-client) before rebasing this PR.

dessalines · 2024-09-19T21:06:10Z

This just needs a new lemmy-js-client deploy then to account for those changes? I've just pushed up 0.20.0-alpha.8 .

EDIT: Oh I gotcha now, I've fixed those broken ones in #2695 . You can either merge from that to pass tests, or just wait until that gets merged soon.

SleeplessOne1917 · 2024-09-20T01:48:39Z

EDIT: Oh I gotcha now, I've fixed those broken ones in #2695 . You can either merge from that to pass tests, or just wait until that gets merged soon.

I'm gonna give this a local lookover once that's done.

privacyguard · 2024-09-20T09:03:03Z

We'll wait until the other PR is merged to complete this PR in one shot.

Co-authored-by: Anthony Lawn <[email protected]>

privacyguard · 2024-09-21T13:21:39Z

@SleeplessOne1917 we integrated the latest fixes from main and passed linting checks. This PR is ready for review.

dessalines · 2024-09-21T14:19:27Z

src/shared/components/home/admin-settings.tsx

@@ -250,7 +258,7 @@ export class AdminSettings extends Component<
                >
                  <div className="row">
                    <TaglineForm
-                      taglines={this.state.siteRes.taglines}
+                      taglines={this.state.siteRes.taglines || []}


Seems like it wasn't necessary.

We added this today because we were getting taglines as undefined and map function was being called on it.

dessalines · 2024-09-21T14:21:15Z

src/shared/components/home/admin-settings.tsx

+      const newOAuthProvider = res.data;
+      this.setState(s => {
+        s.siteRes.admin_oauth_providers = (
+          s.siteRes.admin_oauth_providers || []


A lot of these || [] are probably unecessary.

Both oauth_providers and admin_oauth_providers are optional.

export interface GetSiteResponse { // ... oauth_providers?: Array<PublicOAuthProvider>; admin_oauth_providers?: Array<OAuthProvider>; }

dessalines · 2024-09-21T14:22:47Z

src/shared/components/home/admin-settings.tsx

+        );
+        if (index >= 0) {
+          Object.assign(
+            s.siteRes.admin_oauth_providers[index] || {},


I greatly doubt this Object.assign is necessary. Check out the editListImmutable function we have.

editListImmutable seems to require a key in every object. Example:

let list = [{ provider: { id: 1 } }, { provider: { id: 2 } }]; let item = { provider: { id: 2, new: value } };

In our case, the data structure is:

let siteRes = { admin_oauth_providers: [{id: 1}, {id: 2}] }; let item = { id: 2, new: value };

dessalines · 2024-09-21T14:26:27Z

src/shared/components/home/emojis-form.tsx

-      page: 1 + Math.floor(index / this.itemsPerPage),
-      loading: false,
-    })),
+    customEmojis: (this.isoData.site_res.custom_emojis || []).map(


This change was probably inadvertant also.

This one also we were getting undefined as custom_emojis.

dessalines · 2024-09-21T14:30:16Z

src/shared/components/home/login.tsx

@@ -52,6 +57,9 @@ async function handleLoginSuccess(i: Login, loginRes: LoginResponse) {

  if (site.state === "success") {
    UserService.Instance.myUserInfo = site.data.my_user;
+    const isoData = setIsoData(i.context);
+    isoData.site_res.oauth_providers = site.data.oauth_providers;
+    isoData.site_res.admin_oauth_providers = site.data.admin_oauth_providers;


This is really strange. IsoData is only for the initial fetch, and you should just use siteRes if you need these providers afterward.

It's been a while since we wrote this... We needed to update the the site_res data because the oauth_providers from site_res changes depending on whether the user is logged in as an admin or not.

When you first load the page (not authenticated), you only get the public oauth_provider data needed to display the SSO login buttons. If you log in as an admin, we override this data. If you log out we override the data too.

That said this is not critical data (it does not include any secrets).

src/shared/components/home/login.tsx

dessalines · 2024-09-21T14:35:43Z

src/shared/components/home/oauth-callback.tsx

+  async componentDidMount() {
+    // store state in local storage
+    const local_oauth_state = JSON.parse(
+      localStorage.getItem("oauth_state") || "{}",


Go through and get rid of all these || {}, they're not necessary.

This is different from the rest. In this case, we're checking LocalStorage which could return null. JSON.parse can handle null in javascript but in TS it requires a string.

SleeplessOne1917 · 2024-09-21T16:18:37Z

src/shared/components/common/password-input.tsx

@@ -98,7 +100,7 @@ class PasswordInput extends Component<PasswordInputProps, PasswordInputState> {
                autoComplete={isNew ? "new-password" : "current-password"}
                onInput={onInput}
                value={value}
-                required
+                required={required !== false}


Are you using required !== false because you want it to default to true even if it's undefined?

Correct. We wanted to keep it as required by default and add the ability to disable it when needed.

SleeplessOne1917

In addition to the suggested code changes, I think the UI for adding OAuth providers in the admin settings needs some work. The text inputs are very wide when I don't think they need to be. I think it would be best to display them in 2 columns except on mobile, where they can all be one column.

For the OAuth ID claim, when adding the Privacy Portal preset, the value is "sub". I have no idea what this means. More generally, I think an explanation of what all of those fields do would be helpful, whether that takes the form of help buttons on the input labels that give a description of them, an info box on the page that explains how to set things up, or even a link to a third party resource that can teach the user how to do it.

Finally, for the Oauth Scopes field, I see for the Privacy Portal preset the input has 2 words separated by a space: "openid" and "email". Are these supposed to be selections from a limited set of values? If so, I think a fieldset with checkboxes for those options would be best. If not, I'll have to think of something else.

Since the change to the UI could enc up being a lot, I'll gladly take a stab at it once you implement the code changes me and Dessalines suggested.

src/shared/components/home/login.tsx

src/shared/components/home/oauth-callback.tsx

SleeplessOne1917 · 2024-09-21T16:34:54Z

src/shared/components/home/oauth-callback.tsx

+    if (
+      !this.props.state ||
+      !this.props.code ||
+      !local_oauth_state?.state ||
+      !local_oauth_state?.oauth_provider_id ||
+      !local_oauth_state?.expires_at ||
+      this.props.state !== local_oauth_state.state ||
+      local_oauth_state.expires_at < Date.now()
+    ) {


Given all of the negations, I think it would be good to DeMorgan this:

Suggested change

if (

!this.props.state ||

!this.props.code ||

!local_oauth_state?.state ||

!local_oauth_state?.oauth_provider_id ||

!local_oauth_state?.expires_at ||

this.props.state !== local_oauth_state.state ||

local_oauth_state.expires_at < Date.now()

) {

if (

!(this.props.state &&

this.props.code &&

local_oauth_state?.state &&

local_oauth_state?.oauth_provider_id &&

local_oauth_state?.expires_at &&

this.props.state === local_oauth_state.state) ||

local_oauth_state.expires_at < Date.now()

) {

It feels a bit cleaner not to have a parenthesis. But we can update it if you have a strong opinion on it.

src/shared/components/home/signup.tsx

SleeplessOne1917 · 2024-09-21T17:01:16Z

src/shared/components/home/signup.tsx

+                [
+                  this.titleName(siteView),
+                  ...(oauth_provider !== undefined
+                    ? [`(${oauth_provider.display_name})`]
+                    : []),
+                ].join(" ")


This could be written as:

Suggested change

[

this.titleName(siteView),

...(oauth_provider !== undefined

? [`(${oauth_provider.display_name})`]

: []),

].join(" ")

[

this.titleName(siteView),

...(oauth_provider

? [`(${oauth_provider.display_name})`]

: []),

].join(" ")

SleeplessOne1917 · 2024-09-21T17:02:08Z

src/shared/components/home/signup.tsx

+      return handleUseOAuthProvider({
+        i: undefined,
+        index: undefined,
+        oauth_provider: oauthProvider,
+        username,
+        answer,
+        show_nsfw,
+      });


Can be:

Suggested change

return handleUseOAuthProvider({

i: undefined,

index: undefined,

oauth_provider: oauthProvider,

username,

answer,

show_nsfw,

});

return handleUseOAuthProvider({

oauth_provider: oauthProvider,

username,

answer,

show_nsfw,

});

src/shared/components/home/signup.tsx

src/shared/components/person/settings.tsx

SleeplessOne1917 · 2024-09-22T05:49:01Z

@privacyguard Thanks for doing the code changes. I'm going to be tweaking the UI for this PR a bit and should have my own PR out either Sunday or Monday. I do still need some questions answered. In case you missed them (I made a lot of review comments so they might have gotten buried):

For the OAuth ID claim, when adding the Privacy Portal preset, the value is "sub". I have no idea what this means. More generally, I think an explanation of what all of those fields do would be helpful, whether that takes the form of help buttons on the input labels that give a description of them, an info box on the page that explains how to set things up, or even a link to a third party resource that can teach the user how to do it.

[F]or the Oauth Scopes field, I see for the Privacy Portal preset the input has 2 words separated by a space: "openid" and "email". Are these supposed to be selections from a limited set of values? If so, I think a fieldset with checkboxes for those options would be best. If not, I'll have to think of something else.

privacyguard · 2024-09-22T07:04:53Z

@SleeplessOne1917 perfect.

Concerning the documentation for all the fields: comments from Lemmy.

Concerning the scope, these scopes are semi-standard. Some of them are standard scopes such as openid and others are provider specific. Also the separator of the scopes can differ from one provider to another. The Privacy Portal provider like many others uses spaces as separator. Some providers use commas for example. Please note that in OAUTH the scope is a single string field that includes multiple values (standard and potentially non-standard values).

Concerning the sub, when trying to fetch user data from the provider (using the user-authorized scope), the data is returned as application/json in the provider API response. The Lemmy backend needs to be able to identify which key in this document can be used as a user_id. Many providers, especially those who support OIDC, use the key sub to be consistent with the JWT sub claim used for ids.

SleeplessOne1917 · 2024-09-24T01:39:21Z

While working on this, I came across something about the DTOs that can be tweaked to make it easier for clients: LemmyNet/lemmy#5046

When that PR is merged, I'll generate the types for the JS client and make a PR for that. Then I'll get back to the changes that I made branching off of this PR's branch. The changes I'm making to the frontend diverge from this PR enough that I'll most likely make my PR merge directlly into main instead of the branch this PR uses.

SleeplessOne1917 · 2024-10-02T04:07:25Z

I ended up getting a bit sidetracked on this due to the backend PR for the oauth struct serde serialization and juggling some PRs using the new js client version. With that stuff out of the way, my PR should be up some time this week.

privacyguard · 2024-10-10T16:43:55Z

Closing in favor of SSO Support - alternate UI

privacyguard requested review from dessalines, SleeplessOne1917 and matc-pub as code owners July 2, 2024 21:28

privacyguard changed the title ~~Sso support~~ SSO support Jul 3, 2024

privacyguard mentioned this pull request Jul 3, 2024

SSO Support LemmyNet/lemmy#4881

Merged

privacyguard mentioned this pull request Sep 18, 2024

SSO Support LemmyNet/lemmy-js-client#297

Closed

privacyguard force-pushed the sso_support branch from ff5f058 to 26af93f Compare September 18, 2024 15:24

privacyguard force-pushed the sso_support branch from 26af93f to 7e23d5c Compare September 21, 2024 12:38

OIDC SSO Support

452f7f2

Co-authored-by: Anthony Lawn <[email protected]>

privacyguard force-pushed the sso_support branch from 7e23d5c to 452f7f2 Compare September 21, 2024 13:19

dessalines reviewed Sep 21, 2024

View reviewed changes

SleeplessOne1917 reviewed Sep 21, 2024

View reviewed changes

SleeplessOne1917 requested changes Sep 21, 2024

View reviewed changes

improvements based on review feedback

93620c0

SleeplessOne1917 mentioned this pull request Oct 2, 2024

SSO Support - alternate UI #2731

Merged

privacyguard closed this Oct 10, 2024

privacyguard deleted the sso_support branch October 10, 2024 16:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SSO support #2578

SSO support #2578

privacyguard commented Jul 2, 2024 •

edited

Loading

privacyguard commented Jul 2, 2024

privacyguard commented Sep 18, 2024

dessalines commented Sep 19, 2024 •

edited

Loading

SleeplessOne1917 commented Sep 20, 2024

privacyguard commented Sep 20, 2024

privacyguard commented Sep 21, 2024

dessalines Sep 21, 2024

privacyguard Sep 21, 2024

dessalines Sep 21, 2024

privacyguard Sep 21, 2024

dessalines Sep 21, 2024

privacyguard Sep 21, 2024

dessalines Sep 21, 2024

privacyguard Sep 21, 2024

dessalines Sep 21, 2024

privacyguard Sep 21, 2024

dessalines Sep 21, 2024

privacyguard Sep 21, 2024

SleeplessOne1917 Sep 21, 2024

privacyguard Sep 21, 2024

SleeplessOne1917 left a comment

SleeplessOne1917 Sep 21, 2024

privacyguard Sep 21, 2024

SleeplessOne1917 Sep 21, 2024

privacyguard Sep 21, 2024

SleeplessOne1917 Sep 21, 2024

privacyguard Sep 21, 2024

SleeplessOne1917 commented Sep 22, 2024

privacyguard commented Sep 22, 2024

SleeplessOne1917 commented Sep 24, 2024

SleeplessOne1917 commented Oct 2, 2024

privacyguard commented Oct 10, 2024

SSO support #2578

SSO support #2578

Conversation

privacyguard commented Jul 2, 2024 • edited Loading

How is works?

The usual OIDC endpoints

Who we are? Why are we contributing to Lemmy?

privacyguard commented Jul 2, 2024

privacyguard commented Sep 18, 2024

dessalines commented Sep 19, 2024 • edited Loading

SleeplessOne1917 commented Sep 20, 2024

privacyguard commented Sep 20, 2024

privacyguard commented Sep 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SleeplessOne1917 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SleeplessOne1917 commented Sep 22, 2024

privacyguard commented Sep 22, 2024

SleeplessOne1917 commented Sep 24, 2024

SleeplessOne1917 commented Oct 2, 2024

privacyguard commented Oct 10, 2024

privacyguard commented Jul 2, 2024 •

edited

Loading

dessalines commented Sep 19, 2024 •

edited

Loading