gitea/tea
gitea/tea
56
129
Fork 64
Code Issues 97 Pull Requests 19 Actions Releases 16 Activity

Add support for authentication via ssh certificates and pub/privatekey #442

Merged
6543 merged 11 commits from 42wim/tea:sshcert into main 2022-09-14 19:00:09 +00:00
Conversation 11 Commits 11 Files Changed 6 +262 -40
42wim commented 2021-11-06 19:42:47 +00:00
Member
Copy Link

This adds support for authentication using a SSH certificate and normal public keys when you've got an ssh-agent running that has this certificate or your public key loaded.

First question when creating a new login is to ask about the ssh certificates or public keys, when the answer is yes, we don't need to ask about tokens/usernames anymore.

depends (in order) on:

This adds support for authentication using a SSH certificate and normal public keys when you've got an ssh-agent running that has this certificate or your public key loaded. First question when creating a new login is to ask about the ssh certificates or public keys, when the answer is yes, we don't need to ask about tokens/usernames anymore. depends (in order) on: - https://github.com/go-gitea/gitea/pull/17565 (merged) - https://gitea.com/gitea/go-sdk/pulls/553 (merged)
👍 3 ❤️ 1
42wim added 1 commit 2021-11-06 19:42:48 +00:00
Add support for authentication via ssh certificates
Some checks failed
continuous-integration/drone/pr Build is failing
Details
118a8fefb0
42wim force-pushed sshcert from 118a8fefb0 to ac766ea945 2021-11-06 21:40:20 +00:00 Compare
noerw commented 2021-11-06 21:57:30 +00:00
Member
Copy Link

Nice idea! Some quick remarks, even though the upstream changes are not done:

I'd make the interactive login flow like this:

  • replace the bool prompt do you have a token? with a select prompt login with.. [token/password/sshkey]
    • if loginmethod == ssh_key, prompt for ssh key (ideally also a select populated with ssh-add -l and keys in $HOME/.ssh/*.pub?
    • if not, do sshkey autodetection and prompt for key as already implemented

For non-interactive login, make only one of the set (password, ssh-key, token) required

Also not sure about the new field Login.SSHCert. Is it supposed to store the key material, its fingerprint? We already have Login.SSHKey which contains a path to a key, used for git operations when no ssh-agent auth succeeded.
Would be nice if we could consolidate both fields (but to support the ssh-agent usecase, we'd probably need to migrate away from storing key-paths in SSHKey)

Nice idea! Some quick remarks, even though the upstream changes are not done: I'd make the interactive login flow like this: - replace the bool prompt `do you have a token?` with a select prompt `login with.. [token/password/sshkey]` - if `loginmethod == ssh_key`, prompt for ssh key (ideally also a select populated with `ssh-add -l` and keys in `$HOME/.ssh/*.pub`? - if not, do sshkey autodetection and prompt for key as already implemented For non-interactive login, make only one of the set (password, ssh-key, token) required ~~Also not sure about the new field `Login.SSHCert`. Is it supposed to store the key material, its fingerprint? We already have `Login.SSHKey` which contains a path to a key, used for git operations when no ssh-agent auth succeeded. Would be nice if we could consolidate both fields (but to support the ssh-agent usecase, we'd probably need to migrate away from storing key-paths in `SSHKey`)~~
42wim commented 2021-11-06 22:06:30 +00:00
Author
Member
Copy Link

sshkey and ssh certificate is not the same in this setup.

The sshkey option can only do pull/clone but the ssh certificate has access to the whole API (it talks over http(s), not ssh, it uses the ssh certificate to sign http(s) requests)

The Login.SSHCert is a bool to see if we have it enabled or not, and if so it's given as an option to the NewClient in go-sdk

So I don't think it's a good idea to replace the bool prompt as you suggested. We can change it to: login with.. [token/password/sshkey/sshcert] if you want though.

sshkey and ssh certificate is not the same in this setup. The sshkey option can only do pull/clone but the ssh certificate has access to the whole API (it talks over http(s), not ssh, it uses the ssh certificate to sign http(s) requests) The Login.SSHCert is a bool to see if we have it enabled or not, and if so it's given as an option to the `NewClient` in go-sdk So I don't think it's a good idea to replace the bool prompt as you suggested. We can change it to: `login with.. [token/password/sshkey/sshcert]` if you want though.
noerw commented 2021-11-07 08:13:02 +00:00
Member
Copy Link

Ah I see it now, thanks for clarifying!

Ah I see it now, thanks for clarifying!
noerw changed title from Add support for authentication via ssh certificates to WIP: Add support for authentication via ssh certificates 2021-11-07 08:13:16 +00:00
noerw added the
upstream/gitea
 label 2021-11-07 08:13:34 +00:00
noerw added the
kind
feature
 label 2021-11-07 11:33:35 +00:00
42wim commented 2022-05-29 18:28:03 +00:00
Author
Member
Copy Link

As tea doesn't build against go-sdk master I've made a go-sdk PR againt go-sdk v0.15.0 here: gitea/go-sdk#592

So that this can be tested on current tea

As tea doesn't build against go-sdk master I've made a go-sdk PR againt go-sdk v0.15.0 here: https://gitea.com/gitea/go-sdk/pulls/592 So that this can be tested on current tea
42wim added 1 commit 2022-05-29 19:06:56 +00:00
Merge branch 'master' into sshcert
Some checks failed
continuous-integration/drone/pr Build is failing
Details
3ffef850f8
42wim changed title from WIP: Add support for authentication via ssh certificates to WIP: Add support for authentication via ssh certificates and pub/privatekey 2022-06-14 23:55:40 +00:00
42wim added 2 commits 2022-06-14 23:59:01 +00:00
Use latest go-sdk and bump golang to 1.18 (#485) 
Reviewed-on: #485
Reviewed-by: KN4CK3R <kn4ck3r@noreply.gitea.io>
Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: 6543 <6543@obermui.de>
Co-committed-by: 6543 <6543@obermui.de>
6b0526fc1a
Support public key for httpsign via ssh-agent too
Some checks failed
continuous-integration/drone/pr Build is failing
Details
70e2516f2a
42wim force-pushed sshcert from 70e2516f2a to 1502f26af5 2022-06-15 21:40:24 +00:00 Compare
42wim added 1 commit 2022-06-16 00:27:10 +00:00
Add support for keys outside of ssh-agent and simplify interact menu
Some checks failed
continuous-integration/drone/pr Build is failing
Details
62a335557e
42wim commented 2022-06-16 00:34:29 +00:00
Author
Member
Copy Link

@noerw fyi: I followed your suggestion wrt to the interactive login flow in my latest commit as I'm adding support for normal pubkeys and non-ssh-agent support too.

Had to black out some personal info, but screenshot is clear enough I think

image

image

@noerw fyi: I followed your suggestion wrt to the interactive login flow in my latest commit as I'm adding support for normal pubkeys and non-ssh-agent support too. Had to black out some personal info, but screenshot is clear enough I think ![image](/attachments/f9576042-55ea-4a4c-bf20-95e3e95a7acc) ![image](/attachments/ef616ad8-330a-4d39-9b4b-033e71a8aed4)
image.png
75 KiB
image.png
15 KiB
❤️ 1
42wim added 1 commit 2022-06-16 00:37:34 +00:00
Fix linting issue
Some checks failed
continuous-integration/drone/pr Build is failing
Details
1318ff4e2f
42wim added 1 commit 2022-06-16 00:39:19 +00:00
Fix typo
All checks were successful
continuous-integration/drone/pr Build is passing
Details
7e2edacb4a
42wim added 1 commit 2022-06-16 23:37:01 +00:00
Fix ssh-agent-principal and ssh-agent-key login params
All checks were successful
continuous-integration/drone/pr Build is passing
Details
d6d76ad4cd
42wim added 1 commit 2022-06-17 00:11:31 +00:00
Fix ssh certificate from disk
All checks were successful
continuous-integration/drone/pr Build is passing
Details
16a8dec23d
42wim added 1 commit 2022-06-17 00:21:51 +00:00
Make sshKey option work with certificates
All checks were successful
continuous-integration/drone/pr Build is passing
Details
0744ebc93f
42wim changed title from WIP: Add support for authentication via ssh certificates and pub/privatekey to Add support for authentication via ssh certificates and pub/privatekey 2022-06-17 00:23:25 +00:00
42wim commented 2022-06-17 00:23:52 +00:00
Author
Member
Copy Link

ready for review, hopefully can be released with gitea 1.17 release :)

(ofcourse the go.mod must be changed, but it's included now for easy build/testing)

ready for review, hopefully can be released with gitea 1.17 release :) (ofcourse the go.mod must be changed, but it's included now for easy build/testing)
6543 added this to the v0.9.0 milestone 2022-06-17 00:34:11 +00:00
42wim added 1 commit 2022-06-26 14:42:51 +00:00
Add support for encrypted ssh keys 
People really should use ssh-agent but in the case they don't ask for
the passphrase. We do not save the passphrase, so for every action tea
will ask their passphrase.
All checks were successful
continuous-integration/drone/pr Build is passing
Details
393671f779
42wim commented 2022-06-26 14:50:36 +00:00
Author
Member
Copy Link

Updated to add support for encrypted ssh keys.

Updated to add support for encrypted ssh keys.
6543 reviewed 2022-07-12 16:45:39 +00:00
go.mod Outdated
@ -64,2 +66,4 @@
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c // indirect
)
replace code.gitea.io/sdk/gitea => gitea.com/42wim/go-sdk/gitea v0.0.0-20220624190204-04147197ae82
6543 commented 2022-07-12 16:45:39 +00:00
Owner
Copy Link

sdk pull got merged

sdk pull got merged
42wim commented 2022-07-16 14:29:04 +00:00
Author
Member
Copy Link

ok pulled in upstream go-sdk back

ok pulled in upstream go-sdk back
6543 marked this conversation as resolved
6543 removed the
upstream/gitea
 label 2022-07-12 16:46:25 +00:00
42wim added 1 commit 2022-07-16 14:28:04 +00:00
Use upstream go-sdk
All checks were successful
continuous-integration/drone/pr Build is passing
Details
02e77d463f
6543 approved these changes 2022-07-16 14:39:30 +00:00
42wim commented 2022-08-19 21:00:07 +00:00
Author
Member
Copy Link

Can any of the maintainers can take a look to get this merged ?

Can any of the maintainers can take a look to get this merged ?
👀 1
lunny approved these changes 2022-08-22 01:35:49 +00:00
lunny added 1 commit 2022-08-22 01:36:01 +00:00
Merge branch 'master' into sshcert
Some checks failed
continuous-integration/drone/pr Build is failing
Details
4292e79cbd
6543 modified the milestone from v0.9.0 to v0.10.0 2022-09-13 18:22:28 +00:00
42wim commented 2022-09-14 15:59:18 +00:00
Author
Member
Copy Link

@6543 maintainer edits is now active

@6543 maintainer edits is now active
❤️ 1
42wim added 1 commit 2022-09-14 16:20:51 +00:00
Merge remote-tracking branch 'origin/main' into sshcert
All checks were successful
continuous-integration/drone/pr Build is passing
Details
d66fc0e693
noerw commented 2022-09-14 17:31:25 +00:00
Member
Copy Link

@42wim nice, i'll push some commits later, fixing conflicts and refactoring a bit

@42wim nice, i'll push some commits later, fixing conflicts and refactoring a bit
6543 approved these changes 2022-09-14 18:58:16 +00:00
6543 commented 2022-09-14 18:59:08 +00:00
Owner
Copy Link

@noerw as this pull and your refactoring is in the same milestone ... it wont be breaking change in any case ... so I'll merge it now and you can create a pull targeting main

@noerw as this pull and your refactoring is in the same milestone ... it wont be breaking change in any case ... so I'll merge it now and you can create a pull targeting main
👍 1
6543 merged commit 6a4ba6a689 into main 2022-09-14 19:00:09 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
lunny
6543
Labels
Clear labels   
kind/breaking

   
kind/bug

   
kind/build

   
kind/dependency

   
kind/deployment

   
kind/docs
  
kind
enhancement

  
kind
feature

  
kind/proposal
  
kind
question

  
kind
refactor

  
kind/security

   
kind/testing

   
kind/translation

   
priority/critical

   
priority/high

   
priority/low

   
priority/medium

   
reviewed/duplicate

   
reviewed/invalid

   
reviewed/wontfix

   
skip-changelog

   
status/blocked

   
status/has-backport

   
status/has-pull

   
status/needs-backport

   
status/needs-feedback

   
status/needs-reviews

   
status/wip

   
upstream/gitea

   
upstream/sdk
No Label
kind/breaking
kind/bug
kind/build
kind/dependency
kind/deployment
kind/docs
kind
enhancement
kind
feature
kind/proposal
kind
question
kind
refactor
kind/security
kind/testing
kind/translation
priority/critical
priority/high
priority/low
priority/medium
reviewed/duplicate
reviewed/invalid
reviewed/wontfix
skip-changelog
status/blocked
status/has-backport
status/has-pull
status/needs-backport
status/needs-feedback
status/needs-reviews
status/wip
upstream/gitea
upstream/sdk
Milestone
Clear milestone
No items
No Milestone
v0.10.0
Assignees
Clear assignees
No Assignees
4 Participants
Notifications
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: gitea/tea#442
No description provided.
Delete Branch "42wim/tea:sshcert"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?