Commit c6529456 authored by Georges Racinet's avatar Georges Racinet 🦑
Browse files

Merged upstream v14.10.0 into Heptapod Shell

As usual, including changes in Heptapod version files.

In 3c492987c9aa, upstream fixed the race condition that we
previously had to fix in  cf818cb4e4fe.
Pipeline #58781 passed with stage
in 3 minutes and 51 seconds
v14.10.0
- Implement Push Auth support for 2FA verification !454
v14.9.0
- Update LabKit library to v1.16.0 !668
......
......@@ -10,6 +10,10 @@
The corresponding tag is heptapod-x.y.z
## 14.10.0
- Bump to upstream GitLab Shell v14.10.0 (GitLab 15.3 and 15.4 series)
## 14.9.0
- Bump to upstream GitLab Shell v14.9.0 (GitLab 15.2 series)
......
......@@ -4,6 +4,7 @@
"context"
"fmt"
"io"
"time"
"gitlab.com/gitlab-org/labkit/log"
......@@ -13,6 +14,11 @@
"gitlab.com/gitlab-org/gitlab-shell/v14/internal/gitlabnet/twofactorverify"
)
const (
timeout = 30 * time.Second
prompt = "OTP: "
)
type Command struct {
Config *config.Config
Args *commandargs.Shell
......@@ -20,10 +26,5 @@
}
func (c *Command) Execute(ctx context.Context) error {
ctxlog := log.ContextLogger(ctx)
ctxlog.Info("twofactorverify: execute: waiting for user input")
otp := c.getOTP(ctx)
ctxlog.Info("twofactorverify: execute: verifying entered OTP")
err := c.verifyOTP(ctx, otp)
client, err := twofactorverify.NewClient(c.Config)
if err != nil {
......@@ -29,5 +30,4 @@
if err != nil {
ctxlog.WithError(err).Error("twofactorverify: execute: OTP verification failed")
return err
}
......@@ -31,7 +31,42 @@
return err
}
ctxlog.WithError(err).Info("twofactorverify: execute: OTP verified")
ctx, cancel := context.WithTimeout(ctx, timeout)
defer cancel()
fmt.Fprint(c.ReadWriter.Out, prompt)
resultCh := make(chan string)
go func() {
err := client.PushAuth(ctx, c.Args)
if err == nil {
resultCh <- "OTP has been validated by Push Authentication. Git operations are now allowed."
}
}()
go func() {
answer, err := c.getOTP(ctx)
if err != nil {
resultCh <- formatErr(err)
}
if err := client.VerifyOTP(ctx, c.Args, answer); err != nil {
resultCh <- formatErr(err)
} else {
resultCh <- "OTP validation successful. Git operations are now allowed."
}
}()
var message string
select {
case message = <-resultCh:
case <-ctx.Done():
message = formatErr(ctx.Err())
}
log.WithContextFields(ctx, log.Fields{"message": message}).Info("Two factor verify command finished")
fmt.Fprintf(c.ReadWriter.Out, "\n%v\n", message)
return nil
}
......@@ -35,10 +70,7 @@
return nil
}
func (c *Command) getOTP(ctx context.Context) string {
prompt := "OTP: "
fmt.Fprint(c.ReadWriter.Out, prompt)
func (c *Command) getOTP(ctx context.Context) (string, error) {
var answer string
otpLength := int64(64)
reader := io.LimitReader(c.ReadWriter.In, otpLength)
......@@ -46,12 +78,7 @@
log.ContextLogger(ctx).WithError(err).Debug("twofactorverify: getOTP: Failed to get user input")
}
return answer
}
func (c *Command) verifyOTP(ctx context.Context, otp string) error {
client, err := twofactorverify.NewClient(c.Config)
if err != nil {
return err
if answer == "" {
return "", fmt.Errorf("OTP cannot be blank.")
}
......@@ -56,9 +83,5 @@
}
err = client.VerifyOTP(ctx, c.Args, otp)
if err == nil {
fmt.Fprint(c.ReadWriter.Out, "\nOTP validation successful. Git operations are now allowed.\n")
} else {
fmt.Fprintf(c.ReadWriter.Out, "\nOTP validation failed.\n%v\n", err)
}
return answer, nil
}
......@@ -64,3 +87,4 @@
return nil
func formatErr(err error) string {
return fmt.Sprintf("OTP validation failed: %v", err)
}
......@@ -17,4 +17,13 @@
"gitlab.com/gitlab-org/gitlab-shell/v14/internal/gitlabnet/twofactorverify"
)
type blockingReader struct{}
func (*blockingReader) Read([]byte) (int, error) {
waitInfinitely := make(chan struct{})
<-waitInfinitely
return 0, nil
}
func setup(t *testing.T) []testserver.TestRequestHandler {
......@@ -20,3 +29,4 @@
func setup(t *testing.T) []testserver.TestRequestHandler {
waitInfinitely := make(chan struct{})
requests := []testserver.TestRequestHandler{
{
......@@ -21,6 +31,6 @@
requests := []testserver.TestRequestHandler{
{
Path: "/api/v4/internal/two_factor_otp_check",
Path: "/api/v4/internal/two_factor_manual_otp_check",
Handler: func(w http.ResponseWriter, r *http.Request) {
b, err := io.ReadAll(r.Body)
defer r.Body.Close()
......@@ -31,8 +41,8 @@
require.NoError(t, json.Unmarshal(b, &requestBody))
switch requestBody.KeyId {
case "1":
case "verify_via_otp", "verify_via_otp_with_push_error":
body := map[string]interface{}{
"success": true,
}
json.NewEncoder(w).Encode(body)
......@@ -35,7 +45,9 @@
body := map[string]interface{}{
"success": true,
}
json.NewEncoder(w).Encode(body)
case "wait_infinitely":
<-waitInfinitely
case "error":
body := map[string]interface{}{
"success": false,
......@@ -47,8 +59,32 @@
}
},
},
{
Path: "/api/v4/internal/two_factor_push_otp_check",
Handler: func(w http.ResponseWriter, r *http.Request) {
b, err := io.ReadAll(r.Body)
defer r.Body.Close()
require.NoError(t, err)
var requestBody *twofactorverify.RequestBody
require.NoError(t, json.Unmarshal(b, &requestBody))
switch requestBody.KeyId {
case "verify_via_push":
body := map[string]interface{}{
"success": true,
}
json.NewEncoder(w).Encode(body)
case "verify_via_otp_with_push_error":
w.WriteHeader(http.StatusInternalServerError)
default:
<-waitInfinitely
}
},
},
}
return requests
}
......@@ -50,12 +86,9 @@
}
return requests
}
const (
question = "OTP: \n"
errorHeader = "OTP validation failed.\n"
)
const errorHeader = "OTP validation failed: "
func TestExecute(t *testing.T) {
requests := setup(t)
......@@ -65,7 +98,7 @@
testCases := []struct {
desc string
arguments *commandargs.Shell
answer string
input io.Reader
expectedOutput string
}{
{
......@@ -69,12 +102,27 @@
expectedOutput string
}{
{
desc: "With a known key id",
arguments: &commandargs.Shell{GitlabKeyId: "1"},
answer: "123456\n",
expectedOutput: question +
"OTP validation successful. Git operations are now allowed.\n",
desc: "Verify via OTP",
arguments: &commandargs.Shell{GitlabKeyId: "verify_via_otp"},
expectedOutput: "OTP validation successful. Git operations are now allowed.\n",
},
{
desc: "Verify via OTP",
arguments: &commandargs.Shell{GitlabKeyId: "verify_via_otp_with_push_error"},
expectedOutput: "OTP validation successful. Git operations are now allowed.\n",
},
{
desc: "Verify via push authentication",
arguments: &commandargs.Shell{GitlabKeyId: "verify_via_push"},
input: &blockingReader{},
expectedOutput: "OTP has been validated by Push Authentication. Git operations are now allowed.\n",
},
{
desc: "With an empty OTP",
arguments: &commandargs.Shell{GitlabKeyId: "verify_via_otp"},
input: bytes.NewBufferString("\n"),
expectedOutput: errorHeader + "OTP cannot be blank.\n",
},
{
desc: "With bad response",
arguments: &commandargs.Shell{GitlabKeyId: "-1"},
......@@ -77,10 +125,9 @@
},
{
desc: "With bad response",
arguments: &commandargs.Shell{GitlabKeyId: "-1"},
answer: "123456\n",
expectedOutput: question + errorHeader + "Parsing failed\n",
expectedOutput: errorHeader + "Parsing failed\n",
},
{
desc: "With API returns an error",
arguments: &commandargs.Shell{GitlabKeyId: "error"},
......@@ -83,10 +130,9 @@
},
{
desc: "With API returns an error",
arguments: &commandargs.Shell{GitlabKeyId: "error"},
answer: "yes\n",
expectedOutput: question + errorHeader + "error message\n",
expectedOutput: errorHeader + "error message\n",
},
{
desc: "With API fails",
arguments: &commandargs.Shell{GitlabKeyId: "broken"},
......@@ -89,10 +135,9 @@
},
{
desc: "With API fails",
arguments: &commandargs.Shell{GitlabKeyId: "broken"},
answer: "yes\n",
expectedOutput: question + errorHeader + "Internal API error (500)\n",
expectedOutput: errorHeader + "Internal API error (500)\n",
},
{
desc: "With missing arguments",
arguments: &commandargs.Shell{},
......@@ -95,12 +140,11 @@
},
{
desc: "With missing arguments",
arguments: &commandargs.Shell{},
answer: "yes\n",
expectedOutput: question + errorHeader + "who='' is invalid\n",
expectedOutput: errorHeader + "who='' is invalid\n",
},
}
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
output := &bytes.Buffer{}
......@@ -101,10 +145,14 @@
},
}
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
output := &bytes.Buffer{}
input := bytes.NewBufferString(tc.answer)
input := tc.input
if input == nil {
input = bytes.NewBufferString("123456\n")
}
cmd := &Command{
Config: &config.Config{GitlabUrl: url},
......@@ -115,7 +163,7 @@
err := cmd.Execute(context.Background())
require.NoError(t, err)
require.Equal(t, tc.expectedOutput, output.String())
require.Equal(t, prompt+"\n"+tc.expectedOutput, output.String())
})
}
}
......@@ -119,3 +167,25 @@
})
}
}
func TestCanceledContext(t *testing.T) {
requests := setup(t)
output := &bytes.Buffer{}
url := testserver.StartSocketHttpServer(t, requests)
cmd := &Command{
Config: &config.Config{GitlabUrl: url},
Args: &commandargs.Shell{GitlabKeyId: "wait_infinitely"},
ReadWriter: &readwriter.ReadWriter{Out: output, In: &bytes.Buffer{}},
}
ctx, cancel := context.WithCancel(context.Background())
errCh := make(chan error)
go func() { errCh <- cmd.Execute(ctx) }()
cancel()
require.NoError(t, <-errCh)
require.Equal(t, prompt+"\n"+errorHeader+"context canceled\n", output.String())
}
......@@ -26,7 +26,7 @@
type RequestBody struct {
KeyId string `json:"key_id,omitempty"`
UserId int64 `json:"user_id,omitempty"`
OTPAttempt string `json:"otp_attempt"`
OTPAttempt string `json:"otp_attempt,omitempty"`
}
func NewClient(config *config.Config) (*Client, error) {
......@@ -44,7 +44,22 @@
return err
}
response, err := c.client.Post(ctx, "/two_factor_otp_check", requestBody)
response, err := c.client.Post(ctx, "/two_factor_manual_otp_check", requestBody)
if err != nil {
return err
}
defer response.Body.Close()
return parse(response)
}
func (c *Client) PushAuth(ctx context.Context, args *commandargs.Shell) error {
requestBody, err := c.getRequestBody(ctx, args, "")
if err != nil {
return err
}
response, err := c.client.Post(ctx, "/two_factor_push_otp_check", requestBody)
if err != nil {
return err
}
......
......@@ -17,5 +17,46 @@
)
func initialize(t *testing.T) []testserver.TestRequestHandler {
handler := func(w http.ResponseWriter, r *http.Request) {
b, err := io.ReadAll(r.Body)
defer r.Body.Close()
require.NoError(t, err)
var requestBody *RequestBody
require.NoError(t, json.Unmarshal(b, &requestBody))
switch requestBody.KeyId {
case "0":
body := map[string]interface{}{
"success": true,
}
require.NoError(t, json.NewEncoder(w).Encode(body))
case "1":
body := map[string]interface{}{
"success": false,
"message": "error message",
}
require.NoError(t, json.NewEncoder(w).Encode(body))
case "2":
w.WriteHeader(http.StatusForbidden)
body := &client.ErrorResponse{
Message: "Not allowed!",
}
require.NoError(t, json.NewEncoder(w).Encode(body))
case "3":
w.Write([]byte("{ \"message\": \"broken json!\""))
case "4":
w.WriteHeader(http.StatusForbidden)
}
if requestBody.UserId == 1 {
body := map[string]interface{}{
"success": true,
}
require.NoError(t, json.NewEncoder(w).Encode(body))
}
}
requests := []testserver.TestRequestHandler{
{
......@@ -20,46 +61,11 @@
requests := []testserver.TestRequestHandler{
{
Path: "/api/v4/internal/two_factor_otp_check",
Handler: func(w http.ResponseWriter, r *http.Request) {
b, err := io.ReadAll(r.Body)
defer r.Body.Close()
require.NoError(t, err)
var requestBody *RequestBody
require.NoError(t, json.Unmarshal(b, &requestBody))
switch requestBody.KeyId {
case "0":
body := map[string]interface{}{
"success": true,
}
require.NoError(t, json.NewEncoder(w).Encode(body))
case "1":
body := map[string]interface{}{
"success": false,
"message": "error message",
}
require.NoError(t, json.NewEncoder(w).Encode(body))
case "2":
w.WriteHeader(http.StatusForbidden)
body := &client.ErrorResponse{
Message: "Not allowed!",
}
require.NoError(t, json.NewEncoder(w).Encode(body))
case "3":
w.Write([]byte("{ \"message\": \"broken json!\""))
case "4":
w.WriteHeader(http.StatusForbidden)
}
if requestBody.UserId == 1 {
body := map[string]interface{}{
"success": true,
}
require.NoError(t, json.NewEncoder(w).Encode(body))
}
},
Path: "/api/v4/internal/two_factor_manual_otp_check",
Handler: handler,
},
{
Path: "/api/v4/internal/two_factor_push_otp_check",
Handler: handler,
},
{
Path: "/api/v4/internal/discover",
......@@ -140,6 +146,57 @@
}
}
func TestVerifyPush(t *testing.T) {
client := setup(t)
args := &commandargs.Shell{GitlabKeyId: "0"}
err := client.PushAuth(context.Background(), args)
require.NoError(t, err)
}
func TestErrorMessagePush(t *testing.T) {
client := setup(t)
args := &commandargs.Shell{GitlabKeyId: "1"}
err := client.PushAuth(context.Background(), args)
require.Equal(t, "error message", err.Error())
}
func TestErrorResponsesPush(t *testing.T) {
client := setup(t)
testCases := []struct {
desc string
fakeId string
expectedError string
}{
{
desc: "A response with an error message",
fakeId: "2",
expectedError: "Not allowed!",
},
{
desc: "A response with bad JSON",
fakeId: "3",
expectedError: "Parsing failed",
},
{
desc: "An error response without message",
fakeId: "4",
expectedError: "Internal API error (403)",
},
}
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
args := &commandargs.Shell{GitlabKeyId: tc.fakeId}
err := client.PushAuth(context.Background(), args)
require.EqualError(t, err, tc.expectedError)
})
}
}
func setup(t *testing.T) *Client {
requests := initialize(t)
url := testserver.StartSocketHttpServer(t, requests)
......
......@@ -223,6 +223,5 @@
{"disallowed command", disallowedcommand.Error},
{"not our ref", grpcstatus.Error(grpccodes.Internal, `rpc error: code = Internal desc = cmd wait: exit status 128, stderr: "fatal: git upload-pack: not our ref 9106d18f6a1b8022f6517f479696f3e3ea5e68c1"`)},
} {
ignoredError := ignoredError // Capture range variable, see "Race on loop counter" in https://go.dev/doc/articles/race_detector
t.Run(ignoredError.desc, func(t *testing.T) {
conn, chans = setup(1, newChannel)
......@@ -227,4 +226,5 @@
t.Run(ignoredError.desc, func(t *testing.T) {
conn, chans = setup(1, newChannel)
ignored := ignoredError.err
conn.handleRequests(context.Background(), nil, chans, func(*ssh.ServerConn, ssh.Channel, <-chan *ssh.Request) error {
close(chans)
......@@ -229,6 +229,6 @@
conn.handleRequests(context.Background(), nil, chans, func(*ssh.ServerConn, ssh.Channel, <-chan *ssh.Request) error {
close(chans)
return ignoredError.err
return ignored
})
require.InDelta(t, initialSessionsTotal+2+float64(i), testutil.ToFloat64(metrics.SliSshdSessionsTotal), 0.1)
......
......@@ -11,8 +11,10 @@
'SSH_ORIGINAL_COMMAND' => '2fa_verify' }
end
let(:correct_otp) { '123456' }
before(:context) do