Fix/device login

.github/workflows/golint.yaml

@@ -6,7 +6,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        go-version: ["1.20"]
+        go-version: ["1.21"]
     steps:
 
       - name: Check out code into the Go module directory
@@ -20,5 +20,4 @@ jobs:
       - name: Run golangci-lint
         uses: golangci/golangci-lint-action@v4.0.0
         with:
-          version: v1.51.2
           args: -E bodyclose,gocritic,gofmt,gosec,govet,nestif,nlreturn,revive,rowserrcheck --exclude G401,G501,G107,G307

.github/workflows/integration.yml

@@ -11,7 +11,7 @@ jobs:
       - name: Setup Go
         uses: actions/setup-go@v5
         with:
-          go-version: '1.20'
+          go-version: '1.21'
 
       - name: Set up Python
         uses: actions/setup-python@v5

.github/workflows/test.yml

@@ -9,7 +9,7 @@ jobs:
     runs-on: ${{ matrix.os }}
     strategy:
       matrix:
-        go-version: ["1.20"]
+        go-version: ["1.21"]
         os:
           - ubuntu-latest
           - windows-latest

README.md

@@ -235,7 +235,7 @@ You can get the current version of the sda-cli by running:
 This section contains the information required to install, modify and run the `sda-cli` tool.
 
 ## Requirements
-The `sda-cli` is written in golang. In order to be able to modify, build and run the tool, golang (>= 1.20) needs to be installed. The instructions for installing `go` can be found [here](https://go.dev/doc/install).
+The `sda-cli` is written in golang. In order to be able to modify, build and run the tool, golang (>= 1.21) needs to be installed. The instructions for installing `go` can be found [here](https://go.dev/doc/install).
 
 ## Build tool
 To build the `sda-cli` tool run the following command from the root folder of the repository

datasetsize/datasetsize_test.go

@@ -48,7 +48,7 @@ func (suite *TestSuite) TestFileDoesNotExist() {
 func (suite *TestSuite) TestGetFileSize() {
 	fileContent := "some text!"
 
-	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		_, err := w.Write([]byte(fileContent))
 		assert.NoError(suite.T(), err)
 	}))

download/download_test.go

@@ -101,7 +101,7 @@ func (suite *TestSuite) TestCorrectlyFormatterUrls() {
 
 // Test that the get request doesn't return an error when the server returns 200
 func (suite *TestSuite) TestDownloadFile() {
-	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		w.WriteHeader(http.StatusOK)
 	}))
 	defer ts.Close()
@@ -121,7 +121,7 @@ func (suite *TestSuite) TestdownloadFileErrorStatusCode() {
 	file := "somefile.c4gh"
 
 	// Case when the user tried to download from a private bucket
-	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		w.WriteHeader(http.StatusNotFound)
 		_, _ = io.WriteString(w, "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<Error><Code>NoSuchKey</Code><Message>The specified key does not exist.</Message><Key>A352764B-2KB4-4738-B6B5-BA55D25FB469</Key><BucketName>download</BucketName><Resource>/download/A352764B-2KB4-4738-B6B5-BA55D25FB469</Resource><RequestId>1728F10EAA85663B</RequestId><HostId>73e4c710-46e8-4846-b70b-86ee905a3ab0</HostId></Error>")
 	}))
@@ -131,7 +131,7 @@ func (suite *TestSuite) TestdownloadFileErrorStatusCode() {
 	assert.EqualError(suite.T(), err, "request failed with `404 Not Found`, details: {Code:NoSuchKey Message:The specified key does not exist. Resource:/download/A352764B-2KB4-4738-B6B5-BA55D25FB469}")
 
 	// Case when the user tried to download from a private bucket
-	ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		w.WriteHeader(http.StatusForbidden)
 		_, _ = io.WriteString(w, "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<Error><Code>AllAccessDisabled</Code><Message>All access to this bucket has been disabled.</Message><Resource>/minio/test/dummy/data_file1.c4gh</Resource><RequestId></RequestId><HostId>73e4c710-46e8-4846-b70b-86ee905a3ab0</HostId></Error>")
 	}))
@@ -202,7 +202,7 @@ http://url/to/file2.c4gh
 http://url/to/file3.c4gh
 `
 
-	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		_, err := w.Write([]byte(urlsList))
 		assert.NoError(suite.T(), err)
 	}))

go.mod

@@ -1,6 +1,6 @@
 module github.com/NBISweden/sda-cli
 
-go 1.20
+go 1.21
 
 require (
 	github.com/aws/aws-sdk-go v1.50.30

go.sum

@@ -62,6 +62,7 @@ golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1/go.mod h1:V1LtkGg67GoY2N1AnL
 golang.org/x/net v0.0.0-20190213061140-3a22650c66bd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20190310074541-c10a0554eabf/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.17.0 h1:pVaXccu2ozPjCXewfr1S7xza/zcXTity9cCdXQYSjIM=
+golang.org/x/net v0.17.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE=
 golang.org/x/sys v0.0.0-20181122145206-62eef0e2fa9b/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20200202164722-d101bd2416d5/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
@@ -70,6 +71,7 @@ golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
 golang.org/x/term v0.17.0 h1:mkTF7LCd6WGJNL3K1Ad7kwxNfYAW6a8a8QqtMblp/4U=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ=
+golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU=
 golang.org/x/tools v0.0.0-20190308174544-00c44ba9c14f/go.mod h1:25r3+/G6/xytQM8iWZKq3Hn0kr0rgFKPUNVEL/dr3z4=
 golang.org/x/tools v0.2.0 h1:G6AHpWxTMGY1KyEYoAQ5WTtIekUUvDNjan3ugu60JvE=
 golang.org/x/tools v0.2.0/go.mod h1:y4OqIKeOV/fWJetJ8bXPU1sEVniLMIyDAZWeHdV+NTA=

login/login.go

@@ -1,6 +1,10 @@
 package login
 
 import (
+	"crypto/rand"
+	"crypto/sha256"
+	"encoding/base64"
+	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"flag"
@@ -97,6 +101,7 @@ type DeviceLogin struct {
 	UserInfo        *UserInfo
 	wellKnown       *OIDCWellKnown
 	deviceLogin     *DeviceLoginResponse
+	CodeVerifier    string
 }
 
 type AuthInfo struct {
@@ -108,7 +113,7 @@ type AuthInfo struct {
 
 // requests the /info endpoint to fetch the parameters needed for login
 func GetAuthInfo(baseURL string) (*AuthInfo, error) {
-	url := baseURL + "/info"
+	url := strings.TrimSuffix(baseURL, "/") + "/info"
 	resp, err := http.Get(url)
 	if err != nil {
 		return nil, err
@@ -161,7 +166,7 @@ func (login *DeviceLogin) UpdateConfigFile() error {
 func NewLogin(args []string) error {
 	deviceLogin, err := NewDeviceLogin(args)
 	if err != nil {
-		return fmt.Errorf("failed to contact authentication service")
+		return fmt.Errorf("failed to contact authentication service: %v", err)
 	}
 	err = deviceLogin.Login()
 	if err != nil {
@@ -176,17 +181,17 @@ func NewLogin(args []string) error {
 // `clientID` set.
 func NewDeviceLogin(args []string) (DeviceLogin, error) {
 
-	var url string
+	var loginURL string
 	err := Args.Parse(args[1:])
 	if err != nil {
-		return DeviceLogin{}, errors.New("failed parsing arguments")
+		return DeviceLogin{}, fmt.Errorf("failed parsing arguments: %v", err)
 	}
 	if len(Args.Args()) == 1 {
-		url = Args.Args()[0]
+		loginURL = Args.Args()[0]
 	}
-	info, err := GetAuthInfo(url)
+	info, err := GetAuthInfo(loginURL)
 	if err != nil {
-		return DeviceLogin{}, errors.New("failed to get auth Info")
+		return DeviceLogin{}, fmt.Errorf("failed to get auth Info: %v", err)
 	}
 
 	return DeviceLogin{BaseURL: info.OidcURI, ClientID: info.ClientID, PollingInterval: 2, S3Target: info.InboxURI, PublicKey: info.PublicKey}, nil
@@ -338,8 +343,17 @@ func (login *DeviceLogin) getWellKnown() (*OIDCWellKnown, error) {
 // and sets the login.deviceLogin
 func (login *DeviceLogin) startDeviceLogin() (*DeviceLoginResponse, error) {
 
+	var (
+		err           error
+		codeChallenge string
+	)
+	login.CodeVerifier, codeChallenge, err = generatePKCE(128)
+	if err != nil {
+		return nil, fmt.Errorf("could not create pkce: %v", err)
+	}
+
 	loginBody := fmt.Sprintf("response_type=device_code&client_id=%v"+
-		"&scope=openid ga4gh_passport_v1 profile email", login.ClientID)
+		"&scope=openid ga4gh_passport_v1 profile email&code_challenge_method=S256&code_challenge=%v", login.ClientID, codeChallenge)
 
 	req, err := http.NewRequest("POST",
 		login.wellKnown.DeviceAuthorizationEndpoint, strings.NewReader(loginBody))
@@ -353,18 +367,19 @@ func (login *DeviceLogin) startDeviceLogin() (*DeviceLoginResponse, error) {
 	if err != nil {
 		return nil, err
 	}
-	if resp.StatusCode < 200 || resp.StatusCode >= 400 {
-		err = fmt.Errorf("status code: %v", resp.StatusCode)
-
-		return nil, fmt.Errorf("request failed: %v", err)
-	}
 
 	defer resp.Body.Close()
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return nil, err
 	}
 
+	if resp.StatusCode < 200 || resp.StatusCode >= 400 {
+		err = fmt.Errorf("status code: %v", resp.StatusCode)
+
+		return nil, fmt.Errorf("request failed: %v", err)
+	}
+
 	var loginResponse *DeviceLoginResponse
 	err = json.Unmarshal(body, &loginResponse)
 
@@ -376,7 +391,7 @@ func (login *DeviceLogin) startDeviceLogin() (*DeviceLoginResponse, error) {
 func (login *DeviceLogin) waitForLogin() (*Result, error) {
 
 	body := fmt.Sprintf("grant_type=urn:ietf:params:oauth:grant-type:device_code"+
-		"&client_id=%v&device_code=%v", login.ClientID, login.deviceLogin.DeviceCode)
+		"&client_id=%v&device_code=%v&code_verifier=%v", login.ClientID, login.deviceLogin.DeviceCode, login.CodeVerifier)
 
 	expirationTime := time.Now().Unix() + int64(login.deviceLogin.ExpiresIn)
 
@@ -419,3 +434,24 @@ func (login *DeviceLogin) waitForLogin() (*Result, error) {
 
 	return nil, errors.New("login timed out")
 }
+
+func generatePKCE(count int) (string, string, error) {
+
+	// generate code verifier
+	buf := make([]byte, count)
+	_, err := io.ReadFull(rand.Reader, buf)
+	if err != nil {
+		return "", "", err
+	}
+	verifier := hex.EncodeToString(buf)
+
+	// generate code challenge
+	sha2 := sha256.New()
+	_, err = io.WriteString(sha2, verifier)
+	if err != nil {
+		return "", "", err
+	}
+	challenge := base64.RawURLEncoding.EncodeToString(sha2.Sum(nil))
+
+	return verifier, challenge, nil
+}

testing/README.md

@@ -1,3 +1,24 @@
 # Testing material
 
-All the files included in this folder are support material for the integration tests. Specifically, the `docker-compose.yml` file requires the [S3 proxy](https://github.com/neicnordic/S3-Upload-Proxy) repository in order to run.
+All the files included in this folder are support material for the integration tests.
+
+## How to test the device authorization flow implemented by the login command
+
+From the current (`testing/`) directory and run:
+
+```sh
+export TAG=v0.2.103 && docker compose --profile login up auth oidc --build --force-recreate
+```
+
+and from another terminal:
+
+```sh
+go build .
+./sda-cli login http://localhost:8080
+```
+
+After the login succeeds there should be a `.sda-cli-session` file created in the current directory. To cleanup the environment run:
+
+```sh
+docker compose --profile login down --remove-orphans -v
+```

testing/docker-compose.yml

@@ -101,6 +101,62 @@ services:
       - "5432:5432"
     volumes:
       - dbdata:/var/lib/postgresql/data
+  ## mock oidc server is configured only for device flow,
+  ## the client_id corresponds to the sda-cli client, not sda-auth
+  oidc:
+    profiles: ["login"]
+    container_name: oidc
+    build:
+      context: ./oidc
+      dockerfile: Dockerfile
+    image: mock-oidc-user-server
+    environment:
+      - PORT=9090
+      - HOST=localhost
+      - CLIENT_ID=sda-cli
+      - CLIENT_REDIRECT_URI=http://localhost:8080/elixir/login
+    ports:
+      - 9090:9090
+    healthcheck:
+      test: ["CMD", "curl", "-f", "http://localhost:9090/.well-known/openid-configuration"]
+      interval: 5s
+      timeout: 10s
+      retries: 4
+  keygen:
+    profiles: ["login"]
+    image: golang:alpine3.16
+    container_name: keygen
+    command:
+      - "/bin/sh"
+      - "-c"
+      - if [ ! -f "/out/c4gh.sec.pem" ]; then wget -qO- "https://github.com/neicnordic/crypt4gh/releases/latest/download/crypt4gh_linux_x86_64.tar.gz" | tar zxf -;
+        ./crypt4gh generate -n /shared/c4gh -p privatekeypass; fi;
+    volumes:
+      - shared:/shared
+  ## auth is here only for providing the /info endpoint, no other functionality will work with current configuration
+  auth:
+    profiles: ["login"]
+    container_name: auth
+    image: "ghcr.io/neicnordic/sensitive-data-archive:${TAG}-auth"
+    depends_on:
+      oidc:
+        condition: service_healthy
+      keygen:
+          condition: service_completed_successfully
+    environment:
+      - ELIXIR_ID=sda-cli
+      - ELIXIR_PROVIDER=http://${DOCKERHOST:-localhost}:9090
+      - ELIXIR_SECRET=wHPVQaYXmdDHg  #not used but required so that auth starts
+      - S3INBOX=s3.example.com
+      - PUBLICFILE=/shared/c4gh.pub.pem
+      - RESIGNJWT=false
+    extra_hosts:
+      - ${DOCKERHOST:-localhost}:host-gateway
+    volumes:
+      - shared:/shared
+    ports:
+      - 8080:8080
 volumes:
   data:
   dbdata:
+  shared:

testing/oidc/.dockerignore

@@ -0,0 +1,3 @@
+node_modules
+package-lock.json
+npm-debug.log

testing/oidc/Dockerfile

@@ -0,0 +1,16 @@
+FROM node:16.2.0-slim
+
+WORKDIR /app
+
+RUN apt update && apt upgrade -qy && apt install -qy curl
+
+COPY package.json ./
+
+RUN  rm -rf node_modules
+
+RUN npm install -g npm@latest && \
+    npm i camelcase oidc-provider
+
+COPY . .
+
+CMD [ "node", "server.js" ]

testing/oidc/package.json

@@ -0,0 +1,15 @@
+{
+  "name": "mock-oidc",
+  "version": "0.1.0",
+  "main": "server.js",
+  "engines": {
+    "node": "12"
+  },
+  "scripts": {
+    "start": "node server.js"
+  },
+  "dependencies": {
+    "camelcase": "5.3.1",
+    "oidc-provider": "5.5.2"
+  }
+}