lunny/gitea

Archived

Only show SSH clone URL if signed in (#2169) #2170

Merged

stklcode merged 4 commits from master into master

2017-07-15 14:21:52 +00:00

stklcode commented

2017-07-15 08:06:01 +00:00

(Migrated from github.com)

This PR targets issue #2169

For cloning via SSH a user needs to provide a public key. If this is the case, this user has an account or is added by a member. Anonymous visitors hence do not need to see the SSH URL.

~~This is a hard-coded soliton (as voted by @lafriks), equivalent to GitHub's behavior. If a configurable solution is desired instead, I got another diofferent branch (stklcode/gitea@d7cbf5d72e) ready.~~

This solution introduces a config flag SSH_EXPOSE_ANONYMOUS (default false) to hide by default and opt-in to the old behavior.

This PR targets issue #2169 For cloning via SSH a user needs to provide a public key. If this is the case, this user has an account or is added by a member. Anonymous visitors hence do not need to see the SSH URL. ~~This is a hard-coded soliton (as voted by @lafriks), equivalent to GitHub's behavior. If a configurable solution is desired instead, I got another diofferent branch (stklcode/gitea@d7cbf5d72e15a7411ec1648037fd94872e5ed2a3) ready.~~ This solution introduces a config flag `SSH_EXPOSE_ANONYMOUS` (default `false`) to hide by default and opt-in to the old behavior.

lafriks commented

2017-07-15 08:16:33 +00:00

(Migrated from github.com)

If you have configurable solution than submit that

If you have configurable solution than submit that

stklcode commented

2017-07-15 08:30:12 +00:00

(Migrated from github.com)

Done. Updated my branch and the above description.

Done. Updated my branch and the above description.

lafriks (Migrated from github.com) requested changes 2017-07-15 08:59:43 +00:00

conf/app.ini

lafriks (Migrated from github.com) commented

2017-07-15 08:49:03 +00:00

I think default should be false to match GitHub and also for security reason not to expose unneeded URL's

I think default should be false to match GitHub and also for security reason not to expose unneeded URL's

conf/app.ini

lafriks (Migrated from github.com) commented

2017-07-15 08:48:06 +00:00

Do not change unneeded spaces

Do not change unneeded spaces

modules/setting/setting.go

lafriks (Migrated from github.com) commented

2017-07-15 08:57:42 +00:00

Default value should be better set lower like AuthorizedKeysBackup:
24109f4093/modules/setting/setting.go (L711)

Default value should be better set lower like AuthorizedKeysBackup: https://github.com/stklcode/gitea/blob/24109f4093603a6b4dbcf0735a7506e24ef1e289/modules/setting/setting.go#L711

lafriks commented

2017-07-15 09:04:02 +00:00

(Migrated from github.com)

Also integration test would be nice to check that

Also integration test would be nice to check that

lafriks (Migrated from github.com) approved these changes 2017-07-15 09:31:29 +00:00

lafriks commented

2017-07-15 11:15:56 +00:00

(Migrated from github.com)

I hope you don't mind that I added integration tests, thanks for your first PR 👍

I hope you don't mind that I added integration tests, thanks for your first PR :+1:

lafriks (Migrated from github.com) reviewed 2017-07-15 11:21:41 +00:00

conf/app.ini

lafriks (Migrated from github.com) commented

2017-07-15 11:21:41 +00:00

Please change also comment that default is false

Please change also comment that default is false

lafriks (Migrated from github.com) reviewed 2017-07-15 11:24:12 +00:00

templates/repo/home.tmpl

lafriks (Migrated from github.com) commented

2017-07-15 11:24:12 +00:00

Please also change condition to be same as for button to take into account that ssh is not disabled

Please also change condition to be same as for button to take into account that ssh is not disabled

lafriks (Migrated from github.com) requested changes 2017-07-15 11:31:06 +00:00

templates/repo/home.tmpl

lafriks (Migrated from github.com) commented

2017-07-15 11:30:52 +00:00

Also do not show copy button if both http and ssh clone are disabled or is not signed in and because of that clone url is not visible

Also do not show copy button if both http and ssh clone are disabled or is not signed in and because of that clone url is not visible

lafriks (Migrated from github.com) reviewed 2017-07-15 11:48:42 +00:00

templates/repo/bare.tmpl

lafriks (Migrated from github.com) commented

2017-07-15 11:48:42 +00:00

For bare repository check is not needed as there is already check to show clone url only if user is IsRepositoryAdmin

For bare repository check is not needed as there is already check to show clone url only if user is `IsRepositoryAdmin`

stklcode (Migrated from github.com) reviewed 2017-07-15 12:34:03 +00:00

templates/repo/home.tmpl

stklcode (Migrated from github.com) commented

2017-07-15 12:34:03 +00:00

The button has been shown before, if both were disabled. But you're right, definitely a reasonable change.

The button has been shown before, if both were disabled. But you're right, definitely a reasonable change.

lafriks (Migrated from github.com) approved these changes 2017-07-15 12:37:04 +00:00

lafriks commented

2017-07-15 12:37:52 +00:00

(Migrated from github.com)

Great job 👍 LGTM

Great job :+1: LGTM

Bwko commented

2017-07-15 14:18:58 +00:00

(Migrated from github.com)

LGTM

LGTM

This repo is archived. You cannot comment on pull requests.

No reviewers

No Label

good first issue

hacktoberfest-accepted

kind/deployment

kind/deprecated

kind/enhancement

kind/moderation

kind/regression

kind/translation

kind/upstream-related

performance/bigrepo

performance/cpu

performance/memory

performance/speed

priority/critical

priority/medium

proposal/rejected

reviewed/confirmed

reviewed/duplicate

reviewed/invalid

reviewed/not-a-bug

reviewed/wontfix

status/needs-feedback

theme/authentication

theme/backup-restore

theme/federation

theme/migration

theme/timetracker

No Milestone

No project

No Assignees

1 Participants

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: lunny/gitea#2170

No description provided.