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

[dashboard] Fix rendering of the redirect URI on Git Integrations page #11798

Merged
merged 1 commit into from
Aug 3, 2022

Conversation

AlexTugarev
Copy link
Member

@AlexTugarev AlexTugarev commented Aug 2, 2022

Description

Redirect URI is used to be copy-pasted into OAuth applications of GHE/GL/BBS. When port numbers are included in host names, this was not handled properly in #11409.

Related Issue(s)

Fixes #11795

How to test

Try to create a Git Integration with port number in hostname and see the value of the redirect URI is rendered with colon before the port number.

Release Notes

Fix rendering of the redirect URI on Git Integrations page

Documentation

Werft options:

  • /werft with-preview

@AlexTugarev
Copy link
Member Author

AlexTugarev commented Aug 2, 2022

/werft run

👍 started the job as gitpod-build-alex-git-integrations-shows-wrong-11795.3
(with .werft/ from main)

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-alex-git-integrations-shows-wrong-11795.2 because the annotations in the pull request description changed
(with .werft/ from main)

@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Aug 2, 2022
// `\/` matches the character `/`
// "https://foobar:80".replace(/:(?!\/)/, "_")
// => 'https://foobar_80'
newHostValue = host.replace(/:(?!\/)/, "_");
Copy link
Member Author

Choose a reason for hiding this comment

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

for reviewers, this is running in browser, so it's straight forward to test the example from the comment.

@roboquat roboquat merged commit c31d2fe into main Aug 3, 2022
@roboquat roboquat deleted the alex/git-integrations-shows-wrong-11795 branch August 3, 2022 09:18
@jmls
Copy link

jmls commented Aug 3, 2022

Really needing this 😁 any indication of when it will be live?

@AlexTugarev
Copy link
Member Author

AlexTugarev commented Aug 3, 2022

@jmls, I don't think you need to wait for the next deployment to get unblocked. This PR fixes the rendering of the redirect URI to be entered in your OAuth app on GHE/BBS/GL server, if port number is used.

There is a workaround. You can fix the copy-pasted URL easily yourself. Go to the provider specific OAuth app settings and replace the colon by an underscore, e.g. .../auth/domain.tld:port/callback –> .../auth/domain.tld_port/callback.

@jmls
Copy link

jmls commented Aug 4, 2022

Our customer said that they tried this and it didn't work. I'll have to check again with them

@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Aug 4, 2022
@MaxKienle
Copy link

MaxKienle commented Aug 5, 2022

Hi everybody, just checked with our customer this fix: When entering a URL in the "New Git Integration" Screen with Port and : in the URL the (example foo.bar:7990), the URL will adjusted directly to foo.bar_7990. When trying to activate this connection, we get the error "Host could not be reached"

Could you help us out here?

MicrosoftTeams-image (1)

@jmls
Copy link

jmls commented Aug 9, 2022

@AlexTugarev the behaviour we are seeing is this:

As soon as we try and add the port to the hostname, the colon gets converted to a _. This makes the hostname invalid, and gipod then complains about the host could not be reached. Obviously, the host does not exist with the _

Please could someone actually try this ? It's obvious from the moment you try and press "Aactivaate Integration" that this is flawed and borken

This has been a problem for over 4 weeks now - it's causing major embarrassment with our client

@AlexTugarev
Copy link
Member Author

AlexTugarev commented Aug 9, 2022

@jmls, thanks for the feedback! Let me fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note size/XS team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Git Integrations shows wrong URI if port number is included
5 participants