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

Add possibility to chooses to use https or http for links #699

Open
wants to merge 40 commits into
base: master
Choose a base branch
from

Conversation

EHoglid
Copy link

@EHoglid EHoglid commented Nov 24, 2022

For new features:

  • Describe the use-case in details
    In Sorry Cypress you always assume that the connection to GitLab, GitHub or other web based git platforms are https:// but that is not always the case. If you for example have a self maintained environment then it is possible that this is not configured and you only use http://.
    I have configured a variable USE_SSL_FOR_LINKS that can be set the director part when creating the director in docker.
    'true' = use https://
    'fasle' = use http://

image

Also I discovered that in the dashboard it do the same thing as in the director, converting git@ to https:// so I removed that function handleSshUrl.
The getRemoteOrigin() function now takes a argument if it should return http or https.

  • Ensure the solution is backwards-compatible
    By setting the default value to 'true' this should be backwards-compatible

  • Test it. Submit screenshots / outputs / tests together with the PR.
    Created a own docker and pulled the image and tested it on my computer. (ehoglid/sorry-cypress-....)
    All test passed and extended the test for getRemoteOrigin() to test the extra argument and check if the return is OK.

USE_SSL_FOR_LINKS: 'false'
image

USE_SSL_FOR_LINKS: 'true'
image

No argument in docker, default should be https (backwards-compatible)
image

@EHoglid EHoglid changed the title Sorry cypress Add possibility to chooses to use https or http for links Nov 25, 2022
Copy link
Collaborator

@agoldis agoldis left a comment

Choose a reason for hiding this comment

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

  • Let's rename the variable to "USE_HTTPS_FOR_GIT_ORIGIN"?
  • How about moving the use of USE_SSL_FOR_LINKS to packages/director/src/execution/utils.ts
  • Please update the documentation at https://github.com/sorry-cypress/gitbook

it(`parses remoteOrigin for ${description} correctly`, () => {
const href = getRemoteOrigin(remoteOrigin);
const href = getRemoteOrigin(remoteOrigin, useSSH);
Copy link
Collaborator

Choose a reason for hiding this comment

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

useSSL?


export const getGithubBranchURL = (repo: string, branch: string) =>
handleSshURL(repo.replace(/.git$/, '') + `/tree/${branch}`);

export const handleSshURL = (sshUrl: string) =>
Copy link
Collaborator

Choose a reason for hiding this comment

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

hm.. why was it removed?

Copy link
Author

Choose a reason for hiding this comment

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

What I understand and what I can see in the code this is now handled only in the director converting git URL to a HTTP/HTTPS URL. The dashboard will only get the the Branch URL as either https:// or http://

Example:
director service will convert this data
from:
[email protected]:web-dev/testing_something.git

to:
https://ts-gitlab.sorrycypress.lan/web-dev/testing_something.git

So why do this again in the dashboard?

@mathpaquette
Copy link
Contributor

@EHoglid I dont understand we why need an option for it ? Technically it should be automatically handled no ?

@EHoglid
Copy link
Author

EHoglid commented Jul 11, 2023

@mathpaquette
It depends, normally you can have a settings that redirect http to https if you support https. But in my case our GitLab isn't setup with https, then you can't handle https requests at all.
Second, I have never heard that you redirect a https request to http, that would be a security issue.
Therefor it would be good to choose if it should be http or https.
Of course https is the preferred solution because it is a encrypted connection but in our case it is a local server in a closed environment, therefore it has never been setup.

But I do not see any issue of supporting both? https is the default value anyway, so it won't affect any users if this is implemented.

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

Successfully merging this pull request may close these issues.

None yet

5 participants