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

Extend GitLab sync support to self-hosted instance #874

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MaxSchlueter
Copy link

This is done by getting rid of hard-coded "gitlab.com" URLs, instead
creating a new persistent field that stores the hostname of the GitLab
instance and constructing the URLs based on this field.

Adresses #868

This is done by getting rid of hard-coded "gitlab.com" URLs, instead
creating a new persistent field that stores the hostname of the GitLab
instance and constructing the URLs based on this field.
@chasecaleb
Copy link
Contributor

chasecaleb commented Jul 2, 2022

Hi, do you have a plan for dealing with the OAuth secret? The reason this only works for gitlab.com is because you need a separate OAuth integration for each GitLab instance, so unfortunately you can't just change the URL. The OAuth secret is configured at deploy time, so this won't allow using a different GitLab instance with organice.200ok.ch currently. See https://github.com/200ok-ch/organice/blob/master/.env.sample

@munen
Copy link
Collaborator

munen commented Jul 2, 2022

@chasecaleb Thank you for chiming in🙏

@MaxSchlueter Thank you for giving it a shot and providing a PR🙏To the best of my understanding, @chasecaleb is right. I don't see how to accommodate that requirement on the official instance.

I suppose supporting self-hosted Gitlab instances implies a self-hosted organice instance. If so, we could add a few lines of documentation for it. I suppose that's not so bad. Anyone willing and capable to self-host a non-trivial application as Gitlab is likely capable and willing of self-hosting a single-page application or at least the official Docker image.

@branch14
Copy link
Member

branch14 commented Jul 6, 2022

Here is an idea that I briefly discussed with @munen, and we think it's feasible.

The OAuth secret mentioned by @chasecaleb is currently injected into the build. (In the code it looks like it is read from an env var, but it actually is read from the .env file during build time.) This happens here:

clientId: process.env.REACT_APP_GITLAB_CLIENT_ID,

My suggestion is the following: The sign-in form for Gitlab gets a second input field which takes the OAuth secret (aka Gitlab app token, aka client id). The second input field is disabled as long as the first input field holds a repo url for gitlab.com. If the user enters a repo url for a Gitlab instance other than gitlab.com, the second field is enabled and a token is required.

@chasecaleb What do you think?

@MaxSchlueter Would you like to give this a shot?

@chasecaleb
Copy link
Contributor

Yeah, that's how I would do it too. So long as you think it's worth the complexity then I think that's the right approach.

@MaxSchlueter
Copy link
Author

@MaxSchlueter Would you like to give this a shot?

Sure, thanks for chiming in. I haven't worked with OAuth secrets before, but wouldn't the client secret (REACT_APP_GITLAB_SECRET) as a third input field be needed as well?

@munen
Copy link
Collaborator

munen commented Jul 7, 2022

@MaxSchlueter Yes, both REACT_APP_GITLAB_CLIENT_ID and REACT_APP_GITLAB_SECRET will have to be overridden in the login form.

@kephale
Copy link

kephale commented Aug 27, 2022

I'm super excited about this PR!

@munen
Copy link
Collaborator

munen commented Aug 29, 2022

@kephale Would you like to take it over? There's one small step missing that's discussed starting here: #874 (comment)

@kephale
Copy link

kephale commented Aug 29, 2022

I can check, but it was really easy to just pull these changes into the latest main then (at least) just run off my own instance with my instance's .env info.

It would certainly be better to be able to resolve the secret stuff at runtime instead of build time, but even without that, this solved my use case.

@munen
Copy link
Collaborator

munen commented Aug 30, 2022

It would certainly be better to be able to resolve the secret stuff at runtime instead of build time, but even without that, this solved my use case.

It works for users self-hosting Gitlab and self-hosting organice, now. But not everyone is self-hosting organice. So adding two more fields to the login will be the right way to go forward.

@kephale
Copy link

kephale commented Aug 30, 2022

Some effort in this direction, but not working yet https://github.com/kephale/organice/tree/self-host-wrapup

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.

6 participants