Repository: google/exposure-notifications-server
Pull request #1020 (2020-09-22 02:05:00)

handle nil jwks URI in database
## Proposed Changes

* Handle nil jwks_uri columns in the healthauthority table - migrated rows have a null column value.

**Release Note**

```release-note
bugfix: handle nil JWKS URI values.
```

google-oss-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/google/exposure-notifications-server/pull/1020#" title="Author self-approved">mikehelmick</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=google%2Fexposure-notifications-server).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/google/exposure-notifications-server/blob/main/OWNERS)~~ [mikehelmick]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
google-oss-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/google/exposure-notifications-server/pull/1020#" title="Author self-approved">mikehelmick</a>*, *<a href="https://github.com/google/exposure-notifications-server/pull/1020#pullrequestreview-493088530" title="LGTM">sethvargo</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=google%2Fexposure-notifications-server).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/google/exposure-notifications-server/blob/main/OWNERS)~~ [mikehelmick,sethvargo]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/internal/admin/healthauthority/form.go b/internal/admin/healthauthority/form.go
index 29782d3..7181065 100644
--- a/internal/admin/healthauthority/form.go
+++ b/internal/admin/healthauthority/form.go
@@ -34,7 +34,7 @@ func (f *formData) PopulateHealthAuthority(ha *model.HealthAuthority) {
 	ha.Issuer = f.Issuer
 	ha.Audience = f.Audience
 	ha.Name = f.Name
-	ha.JwksURI = f.JwksURI
+	ha.SetJWKS(f.JwksURI)
 }
 
 type keyFormData struct {
diff --git a/internal/verification/model/health_authority.go b/internal/verification/model/health_authority.go
index 95a25e9..896b80c 100644
--- a/internal/verification/model/health_authority.go
+++ b/internal/verification/model/health_authority.go
@@ -32,7 +32,15 @@ type HealthAuthority struct {
 	Audience string
 	Name     string
 	Keys     []*HealthAuthorityKey
-	JwksURI  string
+	JwksURI  *string
+}
+
+func (ha *HealthAuthority) SetJWKS(uri string) {
+	if uri == "" {
+		ha.JwksURI = nil
+		return
+	}
+	ha.JwksURI = &uri
 }
 
 // Validate returns an error if the HealthAuthority struct is not valid.
diff --git a/pkg/jwks/jwks.go b/pkg/jwks/jwks.go
index 5916e82..a4c261a 100644
--- a/pkg/jwks/jwks.go
+++ b/pkg/jwks/jwks.go
@@ -77,13 +77,17 @@ func NewManager(ctx context.Context, db *database.DB) (*Manager, error) {
 
 // getKeys reads the keys for a single HealthAuthority from its jwks server.
 func (mgr *Manager) getKeys(ctx context.Context, ha *model.HealthAuthority) ([]byte, error) {
-	if len(ha.JwksURI) == 0 {
+	if ha.JwksURI == nil {
+		return nil, nil
+	}
+	jwksURI := *ha.JwksURI
+	if len(jwksURI) == 0 {
 		return nil, nil
 	}
 
 	reqCtxt, done := context.WithTimeout(ctx, 5*time.Second)
 	defer done()
-	req, err := http.NewRequestWithContext(reqCtxt, "GET", ha.JwksURI, nil)
+	req, err := http.NewRequestWithContext(reqCtxt, "GET", jwksURI, nil)
 	if err != nil {
 		return nil, fmt.Errorf("creating connection: %w", err)
 	}
@@ -179,7 +183,7 @@ func findKeyMods(ha *model.HealthAuthority, rxKeys []string) (deadKeys []int, ne
 func (mgr *Manager) updateHA(ctx context.Context, ha *model.HealthAuthority) error {
 	logger := mgr.logger.With("health_authority_name", ha.Name, "health_authority_id", ha.ID)
 
-	if len(ha.JwksURI) == 0 {
+	if ha.JwksURI == nil || len(*ha.JwksURI) == 0 {
 		logger.Infow("skipping jwks, no URI specified")
 		return nil
 	}


Test patch:
diff --git a/internal/verification/database/health_authority_db_test.go b/internal/verification/database/health_authority_db_test.go
index 6721c42..8292fee 100644
--- a/internal/verification/database/health_authority_db_test.go
+++ b/internal/verification/database/health_authority_db_test.go
@@ -42,6 +42,7 @@ func TestAddRetrieveHealthAuthority(t *testing.T) {
 		Issuer:   "doh.mystate.gov",
 		Audience: "ens.usacovid.org",
 		Name:     "My State Department of Healthiness",
+		JwksURI:  nil,
 	}
 
 	haDB := New(testDB)
@@ -79,6 +80,7 @@ func TestAddRetrieveHealthAuthorityKeys(t *testing.T) {
 		Audience: "ens.usacovid.org",
 		Name:     "My State Department of Healthiness",
 	}
+	want.SetJWKS("https://www.example.com/.auth/keys.json")
 
 	haDB := New(testDB)
 	if err := haDB.AddHealthAuthority(ctx, want); err != nil {
diff --git a/pkg/jwks/jwks_test.go b/pkg/jwks/jwks_test.go
index 48547f5..8cab35f 100644
--- a/pkg/jwks/jwks_test.go
+++ b/pkg/jwks/jwks_test.go
@@ -132,7 +132,8 @@ func TestUpdateHA(t *testing.T) {
 			if err != nil {
 				t.Fatalf("[%d] unexpected error: %v", i, err)
 			}
-			ha := &model.HealthAuthority{JwksURI: ts.URL}
+			jwksURI := ts.URL
+			ha := &model.HealthAuthority{JwksURI: &jwksURI}
 
 			// Test networking.
 			rxKeys, err := mgr.getKeys(ctx, ha)
@@ -170,7 +171,7 @@ func TestUpdateHA(t *testing.T) {
 			//
 			// Now test end-to-end.
 			//
-			test.ha.JwksURI = ts.URL
+			test.ha.JwksURI = &jwksURI
 
 			// Add the HealthAuthority & Keys to the DB. Note, we need to remove all
 			// keys from the testing HealthAuthority before adding it to the DB as it's