8000 Feat: add GitLab authentication by almereyda · Pull Request #2555 · outline/outline · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Feat: add GitLab authentication #2555

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
Closed
wants to merge 11 commits into from
Closed

Feat: add GitLab authentication #2555

wants to merge 11 commits into from

Conversation

almereyda
Copy link

This adds GitLab authentication support to Outline.

It is based on #2101 by @ChetanGoti and adds the groups support prototyped in jakoblorz#2.

@auto-assign auto-assign bot requested a review from tommoor September 12, 2021 15:55
@CLAassistant
Copy link
CLAassistant commented Sep 12, 2021 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@almereyda
Copy link
Author

The donation of this PR is a Sunday afternoon's project to congratulate for the latest merge of real-time collaboration.

@almereyda almereyda mentioned this pull request Sep 12, 2021
Closed
@dmezh
Copy link
dmezh commented Oct 3, 2021

You could also accomplish this with GitLab as an OIDC provider. Personally not sure of the utility of maintaining Gitlab as a separate auth strategy besides the groups support, which I think someone was going to add to the OIDC strategy here #2568.

@almereyda
Copy link
Author

The last commit removes the request helper function from the GitLab authentication provider, as proposed and implemented through #2594. From my side I assume this branch is mergeable.

I consider having a native connector useful, due to requiring less boilerplate for a connection between the platforms, and offering the GitLab icon as a hint to the user where they are originating from.

Indeed if #2568 helps to use groups support through the generic OIDC provider already, and helps us to avoid adding another dependency to package.json, I'm happy to follow that route instead. Will we be able to set a custom icon on OIDC Login Buttons?

Additionally, this kind of addon here, one that adds dependencies, might also be a good chance to try out approaches for #2219 instead.

As a side note, if this would be merged, we'd happily run an instance with different Teams for authenticated GitLab groups. Until this is not available on main, it remains a blocker for us.

@EternalDeiwos
Copy link
Contributor

I am not sure if Gitlab provides group information on the JWT; in fact I'm pretty sure they don't, in which case #2568 isn't going to help with that.

@almereyda
Copy link
Author
almereyda commented Nov 11, 2021 •
edited
Loading

I have tested #2568 with a merged main against an existing OIDC connection with GitLab that uses only the openid scope and can confirm that a very small number of the groups associated with the username logging in has indeed been created automatically, yikes!

Apparently there is an error with the group creator that times out

[api] ConnectionAcquireTimeoutError [SequelizeConnectionAcquireTimeoutError]: Operation timeout
[api]     at ConnectionManager.getConnection (/srv/idata/src/github.com/outline/outline/node_modules/sequelize/lib/dialects/abstract/connection-manager.js:288:48)
[api]     at runNextTicks (internal/process/task_queues.js:60:5)
[api]     at listOnTimeout (internal/timers.js:526:9)
[api]     at processTimers (internal/timers.js:500:7)
[api]     at async Transaction.prepareEnvironment (/srv/idata/src/github.com/outline/outline/node_modules/sequelize/lib/transaction.js:119:24)
[api]     at async Sequelize.transaction (/srv/idata/src/github.com/outline/outline/node_modules/sequelize/lib/sequelize.js:1082:7)
[api]     at async groupCreator (/srv/idata/src/github.com/outline/outline/build/server/commands/groupCreator.js:33:23)
[api]     at async Promise.all (index 5)
[api]     at async accountProvisioner (/srv/idata/src/github.com/outline/outline/build/server/commands/accountProvisioner.js:132:14)
[api]     at async OAuth2Strategy._verify (/srv/idata/src/github.com/outline/outline/build/server/routes/auth/providers/oidc.js:100:22) {
[api]   parent: TimeoutError: Operation timeout
[api]       at Timeout.<anonymous> (/srv/idata/src/github.com/outline/outline/node_modules/sequelize-pool/lib/Deferred.js:17:25)
[api]       at listOnTimeout (internal/timers.js:557:17)
[api]       at processTimers (internal/timers.js:500:7),
[api]   original: TimeoutError: Operation timeout
[api]       at Timeout.<anonymous> (/srv/idata/src/github.com/outline/outline/node_modules/sequelize-pool/lib/Deferred.js:17:25)
[api]       at listOnTimeout (internal/timers.js:557:17)
[api]       at processTimers (internal/timers.js:500:7)
[api] }
[api] TypeError: Cannot read property 'map' of undefined
[api]     at accountProvisioner (/srv/idata/src/github.com/outline/outline/build/server/commands/accountProvisioner.js:151:32)
[api]     at runNextTicks (internal/process/task_queues.js:60:5)
[api]     at listOnTimeout (internal/timers.js:526:9)
[api]     at processTimers (internal/timers.js:500:7)
[api]     at async OAuth2Strategy._verify (/srv/idata/src/github.com/outline/outline/build/server/routes/auth/providers/oidc.js:100:22)

but already looks almost usable.

Also it seems that emails are sent synchronously during the callback, but that should be another issue.

Edit: Also, proper groups support would mean we also wanted to enable subdomain groups for self-hosted installs, which is presumably officially not supported.

@almereyda almereyda mentioned this pull request Nov 16, 2021
Closed
@almereyda
Copy link
Author

GitLab can be integrated as an OIDC provider. It does not get the lovely icon, but that's about all that's missing.

@almereyda almereyda closed this Dec 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tommoor tommoor Awaiting requested review from tommoor

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@almereyda @CLAassistant @dmezh @EternalDeiwos @ChetanGoti
0