From 2fe2ae84061ee185e2d16e76e2ca9c4b33d95c83 Mon Sep 17 00:00:00 2001 From: Kian Parvin <46668016+kian99@users.noreply.github.com> Date: Mon, 2 Sep 2024 16:15:07 +0200 Subject: [PATCH 1/7] Fix to rockraft pack action (#1338) --- .github/workflows/release-server-rock.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release-server-rock.yaml b/.github/workflows/release-server-rock.yaml index e07041869..098b9fe84 100644 --- a/.github/workflows/release-server-rock.yaml +++ b/.github/workflows/release-server-rock.yaml @@ -27,7 +27,7 @@ jobs: run: ln -s ./rocks/jimm.yaml ./rockcraft.yaml - name: Build ROCK - uses: canonical/craft-actions/rockcraft-pack@main + uses: canonical/craft-actions/rockcraft-pack@1de2f7678b7ecfd # Fix to build rock due to regression in LXD 5.21 https://discourse.ubuntu.com/t/mount-root-proc-cannot-mount-proc-read-only-with-lxd-5-21-2-22f93f4-from-snap/47533 - name: Load ROCK into local registry run: make load-rock From 11512d7a3b88408428fbb7f81bcffc406ce0ad9c Mon Sep 17 00:00:00 2001 From: Kian Parvin <46668016+kian99@users.noreply.github.com> Date: Mon, 2 Sep 2024 17:29:58 +0200 Subject: [PATCH 2/7] Fix rockcraft CI job (#1341) --- .github/workflows/release-server-rock.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release-server-rock.yaml b/.github/workflows/release-server-rock.yaml index 098b9fe84..4559bbf25 100644 --- a/.github/workflows/release-server-rock.yaml +++ b/.github/workflows/release-server-rock.yaml @@ -27,7 +27,7 @@ jobs: run: ln -s ./rocks/jimm.yaml ./rockcraft.yaml - name: Build ROCK - uses: canonical/craft-actions/rockcraft-pack@1de2f7678b7ecfd # Fix to build rock due to regression in LXD 5.21 https://discourse.ubuntu.com/t/mount-root-proc-cannot-mount-proc-read-only-with-lxd-5-21-2-22f93f4-from-snap/47533 + uses: canonical/craft-actions/rockcraft-pack@1de2f7678b7ecfd491bd8ec3c2d2f73fe722c643 # Fix to build rock due to regression in LXD 5.21 https://discourse.ubuntu.com/t/mount-root-proc-cannot-mount-proc-read-only-with-lxd-5-21-2-22f93f4-from-snap/47533 - name: Load ROCK into local registry run: make load-rock From 769392d0093bccf7a4a1d406267293be11dd2dc5 Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Tue, 3 Sep 2024 09:22:52 +0200 Subject: [PATCH 3/7] Use specific lxd version --- .github/workflows/release-server-rock.yaml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/release-server-rock.yaml b/.github/workflows/release-server-rock.yaml index 4559bbf25..62c698154 100644 --- a/.github/workflows/release-server-rock.yaml +++ b/.github/workflows/release-server-rock.yaml @@ -23,11 +23,19 @@ jobs: username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} + # Fix to build rock due to regression in LXD 5.21/stable https://discourse.ubuntu.com/t/mount-root-proc-cannot-mount-proc-read-only-with-lxd-5-21-2-22f93f4-from-snap/47533 + - name: Setup LXD + uses: canonical/setup-lxd@main + with: + channel: 5.21/candidate + - name: ln rockcraft.yaml run: ln -s ./rocks/jimm.yaml ./rockcraft.yaml - name: Build ROCK - uses: canonical/craft-actions/rockcraft-pack@1de2f7678b7ecfd491bd8ec3c2d2f73fe722c643 # Fix to build rock due to regression in LXD 5.21 https://discourse.ubuntu.com/t/mount-root-proc-cannot-mount-proc-read-only-with-lxd-5-21-2-22f93f4-from-snap/47533 + run: | + /usr/bin/sudo snap install --channel stable --classic rockcraft && \ + rockcraft pack --verbosity trace --use-lxd - name: Load ROCK into local registry run: make load-rock From 392c265dca1451dbb2dd72a83b2551b5eb33ac19 Mon Sep 17 00:00:00 2001 From: Kian Parvin <46668016+kian99@users.noreply.github.com> Date: Tue, 3 Sep 2024 11:20:29 +0200 Subject: [PATCH 4/7] fixes for composite action (#1343) --- .github/actions/test-server/action.yaml | 15 +++++++++++++-- local/jimm/setup-service-account.sh | 2 ++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/.github/actions/test-server/action.yaml b/.github/actions/test-server/action.yaml index 2d2933b36..6878542b9 100644 --- a/.github/actions/test-server/action.yaml +++ b/.github/actions/test-server/action.yaml @@ -41,15 +41,21 @@ runs: username: ${{ github.actor }} password: ${{ inputs.ghcr-pat }} + # We can't use a make target here because a composite action + # doesn't have a .git folder when checked out. - name: Start server based on released version if: ${{ inputs.jimm-version != 'dev' }} - run: make integration-test-env + run: | + cd local/traefik/certs; ./certs.sh; cd - && \ + docker compose --profile test up -d --wait shell: bash + working-directory: ${{ github.action_path }}/../../.. env: JIMM_VERSION: ${{ inputs.jimm-version }} - name: Start server based on development version if: ${{ inputs.jimm-version == 'dev' }} + working-directory: ${{ github.action_path }}/../../.. run: make dev-env shell: bash @@ -59,6 +65,7 @@ runs: echo 'jimm-ca<> $GITHUB_OUTPUT cat ./local/traefik/certs/ca.crt >> $GITHUB_OUTPUT echo 'EOF' >> $GITHUB_OUTPUT + working-directory: ${{ github.action_path }}/../../.. shell: bash - name: Initialise LXD @@ -73,6 +80,7 @@ runs: - name: Setup cloud-init script for bootstraping Juju controllers run: ./local/jimm/setup-controller.sh shell: bash + working-directory: ${{ github.action_path }}/../../.. env: SKIP_BOOTSTRAP: true CLOUDINIT_FILE: "cloudinit.temp.yaml" @@ -83,7 +91,7 @@ runs: provider: "lxd" channel: "5.19/stable" juju-channel: ${{ inputs.juju-channel }} - bootstrap-options: "--config cloudinit.temp.yaml --config login-token-refresh-url=https://jimm.localhost/.well-known/jwks.json" + bootstrap-options: "--config ${{ github.action_path }}/../../../cloudinit.temp.yaml --config login-token-refresh-url=https://jimm.localhost/.well-known/jwks.json" # As described in https://github.com/charmed-kubernetes/actions-operator grab the newly setup controller name - name: Save LXD controller name @@ -100,6 +108,7 @@ runs: - name: Authenticate Juju CLI run: chmod -R 666 ~/.local/share/juju/*.yaml && ./local/jimm/setup-cli-auth.sh + working-directory: ${{ github.action_path }}/../../.. shell: bash # Below is a hardcoded JWT using the same test-secret used in JIMM's docker compose and allows the CLI to authenticate as the jimm-test@canonical.com user. env: @@ -107,6 +116,7 @@ runs: - name: Add LXD Juju controller to JIMM run: ./local/jimm/add-controller.sh + working-directory: ${{ github.action_path }}/../../.. shell: bash env: JIMM_CONTROLLER_NAME: "jimm" @@ -114,4 +124,5 @@ runs: - name: Provide service account with cloud-credentials run: ./local/jimm/setup-service-account.sh + working-directory: ${{ github.action_path }}/../../.. shell: bash diff --git a/local/jimm/setup-service-account.sh b/local/jimm/setup-service-account.sh index b18229a0c..254b379dd 100755 --- a/local/jimm/setup-service-account.sh +++ b/local/jimm/setup-service-account.sh @@ -1,6 +1,7 @@ #!/bin/bash # This script is used to setup a service account by adding a set of cloud-credentials. +# The service account is also made an admin of JIMM. # Default values below assume a lxd controller is added to JIMM. set -eux @@ -11,3 +12,4 @@ CREDENTIAL_NAME="${CREDENTIAL_NAME:-localhost}" juju add-service-account "$SERVICE_ACCOUNT_ID" juju update-service-account-credential "$SERVICE_ACCOUNT_ID" "$CLOUD" "$CREDENTIAL_NAME" +jimmctl auth relation add user-"$SERVICE_ACCOUNT_ID"@serviceaccount administrator controller-jimm From 7734e2203eca1cb86b2ff3894c9174acd0a7b2fe Mon Sep 17 00:00:00 2001 From: Kian Parvin <46668016+kian99@users.noreply.github.com> Date: Wed, 4 Sep 2024 09:57:02 +0200 Subject: [PATCH 5/7] minor repo cleanup (#1340) - Remove old invalid doc - Add more info to charm folder --- .github/workflows/golangci-lint.yaml | 2 +- Makefile | 8 +- charms/README.md | 9 + doc/dev-deploy.md | 276 --------------------------- doc/jimm-facade.md | 3 + 5 files changed, 16 insertions(+), 282 deletions(-) delete mode 100644 doc/dev-deploy.md diff --git a/.github/workflows/golangci-lint.yaml b/.github/workflows/golangci-lint.yaml index 3053f4ec0..9b24d51e7 100644 --- a/.github/workflows/golangci-lint.yaml +++ b/.github/workflows/golangci-lint.yaml @@ -9,7 +9,7 @@ permissions: jobs: golangci: name: Lint - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 steps: - name: Checkout uses: actions/checkout@v4 diff --git a/Makefile b/Makefile index 4dd4e6646..64b6cfe29 100644 --- a/Makefile +++ b/Makefile @@ -62,10 +62,10 @@ simplify: gofmt -w -l -s . # Generate version information -version/commit.txt: FORCE +version/commit.txt: git rev-parse --verify HEAD > version/commit.txt -version/version.txt: FORCE +version/version.txt: if [ -z "$(GIT_VERSION)" ]; then \ echo "dev" > version/version.txt; \ else \ @@ -155,6 +155,4 @@ help: @echo 'make rock - Build the JIMM rock.' @echo 'make load-rock - Load the most recently built rock into your local docker daemon.' -.PHONY: build check install release clean format server simplify sys-deps help FORCE - -FORCE: +.PHONY: build check install release clean format server simplify sys-deps help diff --git a/charms/README.md b/charms/README.md index 5c32b0db8..767d3c083 100644 --- a/charms/README.md +++ b/charms/README.md @@ -1,4 +1,13 @@ # JIMM Charms +Charms are a packaging tool for deploying application using Juju. +See more on charms [here](https://juju.is/charms-architecture). + +JIMM has a machine charm and a Kubernetes charm for deploying the +application on different substrates. Currently the machine charm +is not maintained in favor of the K8s charm. + +The machine charm has not yet been moved to a separate repo. + ## K8S Charm The K8S charm can be found [here](https://github.com/canonical/jimm-k8s-operator/). \ No newline at end of file diff --git a/doc/dev-deploy.md b/doc/dev-deploy.md deleted file mode 100644 index a69824dda..000000000 --- a/doc/dev-deploy.md +++ /dev/null @@ -1,276 +0,0 @@ -# Deploy JAAS - -By following this document you will be able to deploy a complete JAAS -system: Candid with one or more identity providers, JIMM and controllers -added to JIMM. - -## Prerequisites - -### Domain - -To deploy JAAS we need a domain as each service needs a separate DNS entry for each of the deployed service. This will enable us to use -certbot to obtain valid certificates from Let's Encrypt. - -We can use AWS Route 53 to host a subdomain as certbot has a -dns-route53 plugin. To achieve that we need to obtain an -**aws-secret-access-key** and **aws-access-key-id** from Route 53. -In my case i set up AWS Route 53 to manage the canonical.stimec.net -subdomain and i will use this as an example domain for the rest of this -document. - -### Deployment bundles - -Deployment bundles for JAAS can be found in - - git.launchpad.net/canonical-jaas - -You can check it out by running: - - git clone git+ssh://git.launchpad.net/canonical-jaas - -## OpenLDAP - -### Deploy - -We can either bootstrap a new controller or create a new model for the -LDAP deploy: - - juju bootstrap aws dev-ldap - -Then we deploy an ubuntu unit and certbot: - - juju deploy ubuntu ldap - juju deploy cs:~yellow/certbot - -### Install OpenLDAP - -SSH into the unit: - - juju ssh ldap/0 - -Install LDAP by running: - - sudo apt install slapd ldap-utils - -Having installed OpenLDAP run: - - sudo dpkg-reconfigure slapd - -Here i configured LDAP so that admin is: - - cn=admin,dc=ldap,dc=canonical,dc=stimec,dc=net - -And specified a password for the admin user. In the remainder of this -document you will see *dc=ldap, dc=canonical, dc=stimec, dc=net* in -command examples - make sure to replace this with the actual domain you -set up in ldap. - -### Configure TLS - -Then we can exit from the LDAP unit and configure certbot by specifying certificate, key and trust chain paths: - - juju config certbot chain-path=/etc/ldap/ldap-chain.pem - juju config certbot key-path=/etc/ldap/ldap-key.pem - juju config certbot cert-path=/etc/ldap/ldap-cert.pem - -And add a relation between ldap and certbot: - - juju add-relation certbot ldap - -Next we then need to create a DNS **A** record for the LDAP unit. Let's -say we create ldap.<**domain**> that will point to the IP of the -LDAP unit. - -Then, to obtain a certficate we run: - - juju run-action --wait certbot/0 get-certificate agree-tos=true aws-access-key-id=<**aws-secret-access-key-id**> aws-secret-access-key=<**aws-secret-access-key**> - domains=ldap.<**domain**> email=<**developer email**> plugin=dns-route53 - -This will result in creating of .pem files specified by the certbot config -above and we can proceede setting up out LDAP instance. - - juju ssh ldap/0 - - cd /etc/ldap - -Create file certinfo.ldif with the following content - - dn: cn=config - replace: olcTLSCACertificateFile - olcTLSCACertificateFile: /etc/ldap/ldap-chain.pem - - - replace: olcTLSCertificateFile - olcTLSCertificateFile: /etc/ldap/ldap-cert.pem - - - replace: olcTLSCertificateKeyFile - olcTLSCertificateKeyFile: /etc/ldap/ldap-key.pem - -And then change the ldap configuration - - sudo ldapmodify -Y EXTERNAL -H ldapi:/// -f certinfo.ldif - -This will make LDAP use certificates provided by the certbot. - -### LDAP Users - -Then to set up LDAP we create content.ldif with the following content: - - dn: ou=People,dc=ldap,dc=canonical,dc=stimec,dc=net - objectClass: organizationalUnit - ou: People - - dn: ou=Groups,dc=ldap,dc=canonical,dc=stimec,dc=net - objectClass: organizationalUnit - ou: Groups - - dn: cn=miners,ou=Groups,dc=ldap,dc=canonical,dc=stimec,dc=net - objectClass: posixGroup - cn: miners - gidNumber: 5000 - -To create an LDAP user we then create a file named <**username**>.ldif with -the following content: - - dn: uid=<**username**>,ou=People,dc=ldap,dc=canonical,dc=stimec,dc=net - objectClass: inetOrgPerson - objectClass: posixAccount - objectClass: shadowAccount - uid: <**username**> - sn: 2 - givenName: <**name**> - cn: <**username**> - displayName: <**display name**> - uidNumber: <**uuid number e.g. 10000**> - gidNumber: <**gid number e.g. 5000**> - userPassword: {CRYPT}x - gecos: <**display name**> - loginShell: /bin/bash - homeDirectory: /home/<**username**> - -And then run: - - ldapadd -x -D cn=admin,dc=ldap,dc=canonical,dc=stimec,dc=net -W -f <**username**>.ldif - -To set a password for the created user run: - - ldappasswd -x -D cn=admin,dc=ldap,dc=canonical,dc=stimec,dc=net -W -S uid=<**username**>,ou=People,dc=ldap,dc=canonical,dc=stimec,dc=net - -## Candid - -We can either bootstrap a new controller or create a new model for the -Candid deploy: - - juju bootstrap aws dev-candid - -Now go into the folder where you checked out canonical-jaas and into -the /bundles/candid folder. - -To deploy candid run (since we are using certbot to obtain certificates): - - juju deploy ./bundle.yaml --overlay ./overlay-certbot.yaml - -This will deploy 2 candid units, 1 postgresql unit and 2 haproxy units. Feel free to modify the bundle to reduce cpu, memory or root disk contraints. - -Once deployed, we need to create a DNS **A** record for the two haproxy -units - in my case candid.canonical.stimec.net pointed to the one haproxy -unit as haproxy seems to have a problem with peer-to-peer relations that -i couldn't be bothered to resolve. - -Now, to obtain a certificate for haproxy we run: - - - juju run-action --wait certbot/0 get-certificate agree-tos=true aws-access-key-id=<**aws-secret-access-key-id**> aws-secret-access-key=<**aws-secret-access-key**> - domains=candid.<**domain**> email=<**developer email**> plugin=dns-route53 - -Then we need to configure Candid. - -Set the location: - - juju config candid location=https://candid.canonical.stimec.net - -To set up identity providers follow instruction on setting up ldap or -azure then use: - - juju config candid identity-providers= `<**list of identity providers**>` - -### LDAP Identity Provider - -Then we need to set identity-providers configuration option to - - - type: ldap - name: TestLDAP - description: LDAPLogin - domain: ldap.canonical.stimec.net - url: ldap://ldap.canonical.stimec.net/dc=ldap,dc=canonical,dc=stimec,dc=net - dn: cn=admin,dc=ldap,dc=canonical,dc=stimec,dc=net - password: - user-query-filter: (objectClass=inetOrgPerson) - user-query-attrs: - id: uid - email: mail - display-name: displayName - group-query-filter: (&(objectClass=groupOfNames)(member={{.User}})) - hidden: false - ca-cert: | - <> - -### Azure Identity Provider - -If, for some reason you want to add Azure an identity provider go to the -[Azure portal](https://portal.azure.com/) and find **App Registration**. -Fill in the name (e.g. Development Candid). -For **Supported account types** select **Accounts in any organizational directory (Any Azure AD directory - Multitenant) and personal Microsoft accounts (e.g. Skype, Xbox)**. -For **Redirect URI** select **Web** and enter - - https://candid.azure.canonical.com/login/azure/callback - -as the redirect URI. - -Then go to the created **App Registration** and copy **Application (client) ID** (this is **client_id**). The go to **Certificates & secrets**. Create a **New client secret** and copy it's value (this is -**client_secret**). - -Now we need to make Candid aware of the new identity provider. To do that we need to add another item to the identity_providers configuration value. We need to add the following item: - - - type: azure - client-id: <**client_id**> - client-secret: <**client_secret**> - -## JIMM - -We can either bootstrap a new controller or create a new model for the -JIMM deploy: - - juju bootstrap aws dev-jimm - -Now go into the folder where you checked out canonical-jaas and into -the /bundles/jimm folder. - -To deploy jimm run (since we are using certbot to obtain certificates): - - juju deploy ./bundle.yaml --overlay ./overlay-certbot.yaml - -This will deploy 2 jimm units, 1 mongodb unit and 2 haproxy units. Feel free to modify the bundle to reduce cpu, memory or root disk contraints. - -Once deployed, we need to create a DNS **A** record for the two haproxy -units - in my case jimm.canonical.stimec.net pointed to the one haproxy -unit as haproxy seems to have a problem with peer-to-peer relations that -i couldn't be bothered to resolve. - -Now, to obtain a certificate for haproxy we run: - - - juju run-action --wait certbot/0 get-certificate agree-tos=true aws-access-key-id=<**aws-secret-access-key-id**> aws-secret-access-key=<**aws-secret-access-key**> - domains=jimm.<**domain**> email=<**developer email**> plugin=dns-route53 - -Then we need to configure JIMM to use the deployed Candid: - - juju config identity-location=https://candid.<**domain**> - -And add a controller admin, which should be one of the created LDAP users: - - juju config jimm controller-admins=<**username**>@ldap.<**domain**> - -## JIMM Controllers - -To bootstrap and add controllers to JIMM please follow the guide that can -be found [here](https://docs.google.com/document/d/1rtJne7CV6dRsKvCUE85BA5adPvEa5V-TczKSAMzxP9M/edit#heading=h.yij8xaij9lcy). diff --git a/doc/jimm-facade.md b/doc/jimm-facade.md index 96af1872f..c0ffcb15f 100644 --- a/doc/jimm-facade.md +++ b/doc/jimm-facade.md @@ -1,6 +1,9 @@ JIMM Facade =========== +>This document is out of date and requires rework to include details +>on new JIMM specific facades. + In addition to the facades required to emulate a juju controller, JIMM also advertises a JIMM facade with some additional features. From 17bc48050de2c4edf468a6b4eda4ce5e06040f63 Mon Sep 17 00:00:00 2001 From: Kian Parvin <46668016+kian99@users.noreply.github.com> Date: Wed, 4 Sep 2024 16:30:30 +0200 Subject: [PATCH 6/7] Add CORS middleware (#1349) * add cors middleware - fix bug with cookies differing when calling login and whoami endpoints * set httpOnly to true on cookies * add test that CORS is enabled correctly --- cmd/jimmsrv/main.go | 19 +++++++------ cmd/jimmsrv/service/service.go | 23 +++++++++++---- cmd/jimmsrv/service/service_test.go | 44 +++++++++++++++++++++++++++++ go.mod | 1 + go.sum | 4 +-- internal/auth/oauth2.go | 22 +++++++++++++-- internal/auth/oauth2_test.go | 5 ++-- internal/jimmhttp/auth_handler.go | 8 ------ internal/jimmtest/auth.go | 2 +- internal/jujuapi/admin_test.go | 1 + 10 files changed, 100 insertions(+), 29 deletions(-) diff --git a/cmd/jimmsrv/main.go b/cmd/jimmsrv/main.go index 4f5698bd3..efbb80309 100644 --- a/cmd/jimmsrv/main.go +++ b/cmd/jimmsrv/main.go @@ -142,6 +142,8 @@ func start(ctx context.Context, s *service.Service) error { return errors.E("jimm session store secret must be at least 64 characters") } + corsAllowedOrigins := strings.Split(os.Getenv("CORS_ALLOWED_ORIGINS"), " ") + jimmsvc, err := jimmsvc.NewService(ctx, jimmsvc.Params{ ControllerUUID: os.Getenv("JIMM_UUID"), DSN: os.Getenv("JIMM_DSN"), @@ -167,17 +169,18 @@ func start(ctx context.Context, s *service.Service) error { JWTExpiryDuration: jwtExpiryDuration, InsecureSecretStorage: insecureSecretStorage, OAuthAuthenticatorParams: jimmsvc.OAuthAuthenticatorParams{ - IssuerURL: issuerURL, - ClientID: clientID, - ClientSecret: clientSecret, - Scopes: scopesParsed, - SessionTokenExpiry: sessionTokenExpiryDuration, - SessionCookieMaxAge: sessionCookieMaxAgeInt, - JWTSessionKey: sessionSecretKey, + IssuerURL: issuerURL, + ClientID: clientID, + ClientSecret: clientSecret, + Scopes: scopesParsed, + SessionTokenExpiry: sessionTokenExpiryDuration, + SessionCookieMaxAge: sessionCookieMaxAgeInt, + JWTSessionKey: sessionSecretKey, + SecureSessionCookies: secureSessionCookies, }, DashboardFinalRedirectURL: os.Getenv("JIMM_DASHBOARD_FINAL_REDIRECT_URL"), - SecureSessionCookies: secureSessionCookies, CookieSessionKey: []byte(sessionSecretKey), + CorsAllowedOrigins: corsAllowedOrigins, }) if err != nil { return err diff --git a/cmd/jimmsrv/service/service.go b/cmd/jimmsrv/service/service.go index 6ba1dffca..c7e1ead5e 100644 --- a/cmd/jimmsrv/service/service.go +++ b/cmd/jimmsrv/service/service.go @@ -20,6 +20,7 @@ import ( "github.com/juju/names/v5" "github.com/juju/zaputil/zapctx" "github.com/prometheus/client_golang/prometheus/promhttp" + "github.com/rs/cors" "go.uber.org/zap" "gorm.io/driver/postgres" "gorm.io/gorm" @@ -82,6 +83,10 @@ type OAuthAuthenticatorParams struct { // SessionCookieMaxAge holds the max age for session cookies in seconds. SessionCookieMaxAge int + // SecureSessionCookies determines if HTTPS must be enabled in order for JIMM + // to set cookies when creating browser based sessions. + SecureSessionCookies bool + // JWTSessionKey holds the secret key used for signing/verifying JWT tokens. // See internal/auth/oauth2.go AuthenticationService.SessionSecretkey for more details. JWTSessionKey string @@ -175,14 +180,14 @@ type Params struct { // the /callback in an authorisation code OAuth2.0 flow to finish the flow. DashboardFinalRedirectURL string - // SecureSessionCookies determines if HTTPS must be enabled in order for JIMM - // to set cookies when creating browser based sessions. - SecureSessionCookies bool - // CookieSessionKey is a randomly generated secret passed via config used for signing // cookie data. The recommended length is 32/64 characters from the Gorilla securecookie lib. // https://github.com/gorilla/securecookie/blob/main/securecookie.go#L124 CookieSessionKey []byte + + // CorsAllowedOrigins represents all addresses that are valid for cross-origin + // requests. A wildcard '*' is accepted to allow all cross-origin requests. + CorsAllowedOrigins []string } // A Service is the implementation of a JIMM server. @@ -347,6 +352,7 @@ func NewService(ctx context.Context, p Params) (*Service, error) { SessionTokenExpiry: p.OAuthAuthenticatorParams.SessionTokenExpiry, SessionCookieMaxAge: p.OAuthAuthenticatorParams.SessionCookieMaxAge, JWTSessionKey: p.OAuthAuthenticatorParams.JWTSessionKey, + SecureCookies: p.OAuthAuthenticatorParams.SecureSessionCookies, Store: &s.jimm.Database, SessionStore: sessionStore, RedirectURL: redirectUrl, @@ -380,6 +386,14 @@ func NewService(ctx context.Context, p Params) (*Service, error) { return nil, errors.E(op, err, "failed to parse final redirect url for the dashboard") } + // Setup CORS middleware + corsOpts := cors.New(cors.Options{ + AllowedOrigins: p.CorsAllowedOrigins, + AllowedMethods: []string{"GET"}, + AllowCredentials: true, + }) + s.mux.Use(corsOpts.Handler) + // Setup all HTTP handlers. mountHandler := func(path string, h jimmhttp.JIMMHttpHandler) { s.mux.Mount(path, h.Routes()) @@ -406,7 +420,6 @@ func NewService(ctx context.Context, p Params) (*Service, error) { oauthHandler, err := jimmhttp.NewOAuthHandler(jimmhttp.OAuthHandlerParams{ Authenticator: authSvc, DashboardFinalRedirectURL: p.DashboardFinalRedirectURL, - SecureCookies: p.SecureSessionCookies, }) if err != nil { zapctx.Error(ctx, "failed to setup authentication handler", zap.Error(err)) diff --git a/cmd/jimmsrv/service/service_test.go b/cmd/jimmsrv/service/service_test.go index 9bc815d69..3d35384bc 100644 --- a/cmd/jimmsrv/service/service_test.go +++ b/cmd/jimmsrv/service/service_test.go @@ -8,6 +8,7 @@ import ( "io" "net/http" "net/http/httptest" + "net/url" "os" "testing" @@ -483,3 +484,46 @@ func TestCleanupDoesNotPanic_SessionStoreRelatedCleanups(t *testing.T) { svc.Cleanup() } + +func TestCORS(t *testing.T) { + c := qt.New(t) + + _, _, cofgaParams, err := jimmtest.SetupTestOFGAClient(c.Name()) + c.Assert(err, qt.IsNil) + p := jimmtest.NewTestJimmParams(c) + p.OpenFGAParams = cofgaParamsToJIMMOpenFGAParams(*cofgaParams) + allowedOrigin := "http://my-referrer.com" + p.CorsAllowedOrigins = []string{allowedOrigin} + p.InsecureSecretStorage = true + + svc, err := jimmsvc.NewService(context.Background(), p) + c.Assert(err, qt.IsNil) + defer svc.Cleanup() + + srv := httptest.NewServer(svc) + c.Cleanup(srv.Close) + + url, err := url.Parse(srv.URL + "/debug/info") + c.Assert(err, qt.IsNil) + // Invalid origin won't receive CORS headers. + req := http.Request{ + Method: "GET", + URL: url, + Header: http.Header{"Origin": []string{"123"}}, + } + response, err := srv.Client().Do(&req) + c.Assert(err, qt.IsNil) + defer response.Body.Close() + c.Assert(response.StatusCode, qt.Equals, http.StatusOK) + c.Assert(response.Header.Get("Access-Control-Allow-Credentials"), qt.Equals, "") + c.Assert(response.Header.Get("Access-Control-Allow-Origin"), qt.Equals, "") + + // Valid origin should receive CORS headers. + req.Header = http.Header{"Origin": []string{allowedOrigin}} + response, err = srv.Client().Do(&req) + c.Assert(err, qt.IsNil) + defer response.Body.Close() + c.Assert(response.StatusCode, qt.Equals, http.StatusOK) + c.Assert(response.Header.Get("Access-Control-Allow-Credentials"), qt.Equals, "true") + c.Assert(response.Header.Get("Access-Control-Allow-Origin"), qt.Equals, allowedOrigin) +} diff --git a/go.mod b/go.mod index 30ba26e37..58a2c71b0 100644 --- a/go.mod +++ b/go.mod @@ -58,6 +58,7 @@ require ( github.com/lestrrat-go/iter v1.0.2 github.com/lestrrat-go/jwx/v2 v2.0.21 github.com/oklog/ulid/v2 v2.1.0 + github.com/rs/cors v1.11.1 github.com/stretchr/testify v1.9.0 golang.org/x/oauth2 v0.16.0 gopkg.in/errgo.v1 v1.0.1 diff --git a/go.sum b/go.sum index e742a5d03..5093613c6 100644 --- a/go.sum +++ b/go.sum @@ -956,8 +956,8 @@ github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFR github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8= github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4= -github.com/rs/cors v1.10.1 h1:L0uuZVXIKlI1SShY2nhFfo44TYvDPQ1w4oFkUJNfhyo= -github.com/rs/cors v1.10.1/go.mod h1:XyqrcTp5zjWr1wsJ8PIRZssZ8b/WMcMf71DJnit4EMU= +github.com/rs/cors v1.11.1 h1:eU3gRzXLRK57F5rKMGMZURNdIG4EoAmX8k94r9wXWHA= +github.com/rs/cors v1.11.1/go.mod h1:XyqrcTp5zjWr1wsJ8PIRZssZ8b/WMcMf71DJnit4EMU= github.com/rs/xid v1.2.1/go.mod h1:+uKXf+4Djp6Md1KODXJxgGQPKngRmWyn10oCKFzNHOQ= github.com/rs/xid v1.5.0 h1:mKX4bl4iPYJtEIxp6CYiUuLQ/8DYMoz0PUdtGgMFRVc= github.com/rs/xid v1.5.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg= diff --git a/internal/auth/oauth2.go b/internal/auth/oauth2.go index feff81490..f68017734 100644 --- a/internal/auth/oauth2.go +++ b/internal/auth/oauth2.go @@ -76,6 +76,8 @@ type AuthenticationService struct { sessionTokenExpiry time.Duration // sessionCookieMaxAge holds the max age for session cookies in seconds. sessionCookieMaxAge int + // secureCookies decides whether to set the secure flag on cookies. + secureCookies bool // jwtSessionKey holds the secret key used for signing/verifying JWT tokens. // According to https://datatracker.ietf.org/doc/html/rfc7518 minimum key lengths are // HSXXX e.g. HS256 - 256 bits, RSA - at least 2048 bits. @@ -119,6 +121,9 @@ type AuthenticationServiceParams struct { // SessionCookieMaxAge holds the max age for session cookies in seconds. SessionCookieMaxAge int + // SecureCookies decides whether to set the secure flag on cookies. + SecureCookies bool + // JWTSessionKey holds the secret key used for signing/verifying JWT tokens. // See AuthenticationService.JWTSessionKey for more details. JWTSessionKey string @@ -163,6 +168,7 @@ func NewAuthenticationService(ctx context.Context, params AuthenticationServiceP db: params.Store, sessionStore: params.SessionStore, sessionCookieMaxAge: params.SessionCookieMaxAge, + secureCookies: params.SecureCookies, }, nil } @@ -420,13 +426,23 @@ func (as *AuthenticationService) VerifyClientCredentials(ctx context.Context, cl return nil } +// sessionCrossOriginSafe sets parameters on the session that allow its use in cross-origin requests. +// Options are not saved to the database so this must be called whenever a session cookie will be returned to a client. +// +// Note browsers require cookies with the same-site policy as 'none' to additionally have the secure flag set. +func sessionCrossOriginSafe(session *sessions.Session, secure bool) *sessions.Session { + session.Options.Secure = secure // Ensures only sent with HTTPS + session.Options.HttpOnly = true // Don't allow Javascript to modify cookie + session.Options.SameSite = http.SameSiteNoneMode // Allow cross-origin requests via Javascript + return session +} + // CreateBrowserSession creates a session and updates the cookie for a browser // login callback. func (as *AuthenticationService) CreateBrowserSession( ctx context.Context, w http.ResponseWriter, r *http.Request, - secureCookies bool, email string, ) error { const op = errors.Op("auth.AuthenticationService.CreateBrowserSession") @@ -438,8 +454,7 @@ func (as *AuthenticationService) CreateBrowserSession( session.IsNew = true // Sets cookie to a fresh new cookie session.Options.MaxAge = as.sessionCookieMaxAge // Expiry in seconds - session.Options.Secure = secureCookies // Ensures only sent with HTTPS - session.Options.HttpOnly = false // Allow Javascript to read it + session = sessionCrossOriginSafe(session, as.secureCookies) session.Values[SessionIdentityKey] = email if err = session.Save(r, w); err != nil { @@ -465,6 +480,7 @@ func (as *AuthenticationService) AuthenticateBrowserSession(ctx context.Context, if err != nil { return ctx, errors.E(op, err, "failed to retrieve session") } + session = sessionCrossOriginSafe(session, as.secureCookies) identityId, ok := session.Values[SessionIdentityKey] if !ok { diff --git a/internal/auth/oauth2_test.go b/internal/auth/oauth2_test.go index fa6ac6999..9de52948f 100644 --- a/internal/auth/oauth2_test.go +++ b/internal/auth/oauth2_test.go @@ -50,6 +50,7 @@ func setupTestAuthSvc(ctx context.Context, c *qt.C, expiry time.Duration) (*auth SessionStore: sessionStore, SessionCookieMaxAge: 60, JWTSessionKey: "secret-key", + SecureCookies: false, }) c.Assert(err, qt.IsNil) cleanup := func() { @@ -295,7 +296,7 @@ func TestCreateBrowserSession(t *testing.T) { req, err := http.NewRequest("GET", "", nil) c.Assert(err, qt.IsNil) - err = authSvc.CreateBrowserSession(ctx, rec, req, false, "jimm-test@canonical.com") + err = authSvc.CreateBrowserSession(ctx, rec, req, "jimm-test@canonical.com") c.Assert(err, qt.IsNil) cookies := rec.Header().Get("Set-Cookie") @@ -508,6 +509,6 @@ func TestAuthenticateBrowserSessionHandlesMissingOrExpiredRefreshTokens(t *testi c.Assert( setCookieCookies, qt.Equals, - "jimm-browser-session=; Path=/; Expires=Thu, 01 Jan 1970 00:00:01 GMT; Max-Age=0", + "jimm-browser-session=; Path=/; Expires=Thu, 01 Jan 1970 00:00:01 GMT; Max-Age=0; HttpOnly; SameSite=None", ) } diff --git a/internal/jimmhttp/auth_handler.go b/internal/jimmhttp/auth_handler.go index d8590122a..285093a84 100644 --- a/internal/jimmhttp/auth_handler.go +++ b/internal/jimmhttp/auth_handler.go @@ -34,7 +34,6 @@ type OAuthHandler struct { Router *chi.Mux authenticator BrowserOAuthAuthenticator dashboardFinalRedirectURL string - secureCookies bool } // OAuthHandlerParams holds the parameters to configure the OAuthHandler. @@ -45,10 +44,6 @@ type OAuthHandlerParams struct { // DashboardFinalRedirectURL is the final redirection URL to send users to // upon completing the authorisation code flow. DashboardFinalRedirectURL string - - // SessionCookies determines if HTTPS must be enabled in order for JIMM - // to set cookies when creating browser based sessions. - SecureCookies bool } // BrowserOAuthAuthenticator handles authorisation code authentication within JIMM @@ -63,7 +58,6 @@ type BrowserOAuthAuthenticator interface { ctx context.Context, w http.ResponseWriter, r *http.Request, - secureCookies bool, email string, ) error Logout(ctx context.Context, w http.ResponseWriter, req *http.Request) error @@ -83,7 +77,6 @@ func NewOAuthHandler(p OAuthHandlerParams) (*OAuthHandler, error) { Router: chi.NewRouter(), authenticator: p.Authenticator, dashboardFinalRedirectURL: p.DashboardFinalRedirectURL, - secureCookies: p.SecureCookies, }, nil } @@ -173,7 +166,6 @@ func (oah *OAuthHandler) Callback(w http.ResponseWriter, r *http.Request) { ctx, w, r, - oah.secureCookies, email, ); err != nil { writeError(ctx, w, http.StatusInternalServerError, err, "failed to setup session") diff --git a/internal/jimmtest/auth.go b/internal/jimmtest/auth.go index 2eb24d476..6a890dfcc 100644 --- a/internal/jimmtest/auth.go +++ b/internal/jimmtest/auth.go @@ -239,6 +239,7 @@ func SetupTestDashboardCallbackHandler(browserURL string, db *db.Database, sessi SessionStore: sessionStore, SessionCookieMaxAge: 60, JWTSessionKey: "test-secret", + SecureCookies: false, }) if err != nil { return nil, err @@ -247,7 +248,6 @@ func SetupTestDashboardCallbackHandler(browserURL string, db *db.Database, sessi h, err := jimmhttp.NewOAuthHandler(jimmhttp.OAuthHandlerParams{ Authenticator: authSvc, DashboardFinalRedirectURL: browserURL, - SecureCookies: false, }) if err != nil { return nil, err diff --git a/internal/jujuapi/admin_test.go b/internal/jujuapi/admin_test.go index 94e2db196..f306a58c9 100644 --- a/internal/jujuapi/admin_test.go +++ b/internal/jujuapi/admin_test.go @@ -62,6 +62,7 @@ func (s *adminSuite) SetUpTest(c *gc.C) { SessionStore: sessionStore, SessionCookieMaxAge: 60, JWTSessionKey: "test-secret", + SecureCookies: false, }) c.Assert(err, gc.Equals, nil) s.JIMM.OAuthAuthenticator = authSvc From 5d2faa0191d292042c1818c868c0f2f33518e395 Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Wed, 4 Sep 2024 16:51:43 +0200 Subject: [PATCH 7/7] fix missing docstring --- internal/jimm/admin.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/jimm/admin.go b/internal/jimm/admin.go index f17ff3690..c714e35fd 100644 --- a/internal/jimm/admin.go +++ b/internal/jimm/admin.go @@ -23,6 +23,7 @@ func (j *JIMM) LoginDevice(ctx context.Context) (*oauth2.DeviceAuthResponse, err return resp, nil } +// AuthenticateBrowserSession authenticates a browser login. func (j *JIMM) AuthenticateBrowserSession(ctx context.Context, w http.ResponseWriter, r *http.Request) (context.Context, error) { return j.OAuthAuthenticator.AuthenticateBrowserSession(ctx, w, r) }