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

feat: add GitLab OAuth client #4692

Merged

Conversation

almereyda
Copy link
Contributor

@almereyda almereyda commented Jun 3, 2024

This adds a GitLab OAuth client, based on the Google and GitHub examples.

To raise exceptions when OAuth is malconfigured, it also adds an additional OAUTH_NOT_CONFIGURED error.

There is a bit of code duplication between admin, web and spaces, but that may be a design choice.

This has been tested successfully against a custom GitLab instance and should work similarly against the default instance.

addresses #408
addresses #413

@CLAassistant
Copy link

CLAassistant commented Jun 3, 2024

CLA assistant check
All committers have signed the CLA.

@almereyda
Copy link
Contributor Author

almereyda commented Jun 4, 2024

As it turns out, I didn't find a place in the repository to document the added environment variables.

Seems good to accompany this PR with a parallel one in makeplane/docs.

@almereyda almereyda force-pushed the feat/gitlab-oauth branch from 9ae2b7e to 59b86e9 Compare June 13, 2024 15:17
@almereyda
Copy link
Contributor Author

Rebased to accommodate 36b82a7

@almereyda almereyda force-pushed the feat/gitlab-oauth branch from 59b86e9 to b33ed0b Compare June 13, 2024 15:41
@sriramveeraghanta sriramveeraghanta added this to the v0.22-dev milestone Jun 14, 2024
@sriramveeraghanta sriramveeraghanta changed the base branch from preview to gitlab-oauth June 14, 2024 09:22
@sriramveeraghanta sriramveeraghanta merged commit 99e1963 into makeplane:gitlab-oauth Jun 14, 2024
12 of 13 checks passed
@sriramveeraghanta
Copy link
Contributor

@almereyda

Thank you for your contribution. We have merged your pull request into an internal branch for validation and testing. We will soon merge these changes into the master branch along with the updates for our next release.

@almereyda
Copy link
Contributor Author

Thanks! I'm very excited.

Please test things out. The last rebase unfortunately removed the three commit version and put everything into a single commit again. I'm worried I've rebased the wrong branch, but hope I've catched the correct state of the implementation.

Please let me know if you run into any issues.

@almereyda
Copy link
Contributor Author

almereyda commented Jun 14, 2024

My intuition was correct. In trying to react quick with a rebase, I didn't fetch my force-pushed working branch from the other machine, which resulted in me pushing an outdated state of the PR.

Thank you for being so diligent in checking this out into an internal testing branch.

Please reset it hard by HEAD~1 and replace it with what is presented in:

🙈

@sriramveeraghanta
Copy link
Contributor

Our team will take over from here. We will test it to ensure everything is working and address any issues that arise during the testing process.

@almereyda
Copy link
Contributor Author

Thanks for the notice. Feel free to reuse, recommit or remix the content of the PRs in any way.

Also feel free to close #4828, if suitable.

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

Successfully merging this pull request may close these issues.

4 participants