Repository: TBD54566975/ssi-service (go)

Issue:
Passing no expiration to v1/presentations/request should not set a default expiration
There has been discussion of why the current behavior is to set a default expiration on the presentation request that's being created. It's proposed to not set expiration in the presentation request JWT.

See https://discord.com/channels/937858703112155166/969272692891086868/1121290626999263333 and the subsequent messages. 

The summary for reasons to adopt not setting the `exp` claim in the JWT: 
* The use of `exp` is optional in the JWT spec.
* When the presentation request is meant to be static and not change, it doesn't make sense to expire it.
* Imposing expiration as a requirement ends up becoming a burden.

The summary for reasons for always setting the `exp` claim in the JWT that represents the request.
* It acts as a mitigation in case that the presentation request needs to be invalidated.
  * This is not preventing an attack vector, but it's mitigation if an attack happens (I had communicated this incorrectly in the past).
* We don't expose expiration functionality of requests in the server side.
* Presentation requests will support replay protection in the future (this is NOT supported right now). The current thinking is that replay protection should be enabled by default. For single use presentation requests, including the expiration date is a good practice. An example of this is the NIST recommendation for address confirmation. See Section 4.4.1.6, item 5.e of https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-63a.pdf. The equivalent of enrollment codes are `nonce`. 


My personal take
* While I believe that "presentation request is meant to be static and not change" will end up actually changing, I think we don't know yet what a "best practice" will be related to Presentation Requests. So I think we should meet the current needs and figure it out from there. 
* Additionally, we can make it a RECOMMENDATION, instead of a MUST.

Pull request:
makes expiration optional for request endpoints
# Overview
Fixes #557. This makes expiration optional for any requests that are created. 

# Description
Changes a default behavior according to the issue.

# How Has This Been Tested?
Unit and integration tests pass. 


Patch:
diff --git a/config/config.go b/config/config.go
index ed709e8..847976f 100644
--- a/config/config.go
+++ b/config/config.go
@@ -171,7 +171,6 @@ func (o *OperationServiceConfig) IsEmpty() bool {
 
 type PresentationServiceConfig struct {
 	*BaseServiceConfig
-	ExpirationDuration time.Duration `toml:"expiration_duration" conf:"default:30m"`
 }
 
 func (p *PresentationServiceConfig) IsEmpty() bool {
@@ -183,7 +182,6 @@ func (p *PresentationServiceConfig) IsEmpty() bool {
 
 type ManifestServiceConfig struct {
 	*BaseServiceConfig
-	ExpirationDuration time.Duration `toml:"expiration_duration" conf:"default:30m"`
 }
 
 func (m *ManifestServiceConfig) IsEmpty() bool {
diff --git a/config/config.toml.example b/config/config.toml.example
index b85c536..835aa84 100644
--- a/config/config.toml.example
+++ b/config/config.toml.example
@@ -71,7 +71,6 @@ name = "manifest"
 
 [services.presentation]
 name = "presentation"
-expiration_duration = "30m"
 
 [services.webhook]
 name = "webhook"
diff --git a/config/dev.toml b/config/dev.toml
index 076c027..952a9c9 100644
--- a/config/dev.toml
+++ b/config/dev.toml
@@ -61,7 +61,6 @@ name = "manifest"
 
 [services.presentation]
 name = "presentation"
-expiration_duration = "30m"
 
 [services.webhook]
 name = "webhook"
diff --git a/config/prod.toml b/config/prod.toml
index ca831e7..a962017 100644
--- a/config/prod.toml
+++ b/config/prod.toml
@@ -66,7 +66,6 @@ name = "manifest"
 
 [services.presentation]
 name = "presentation"
-expiration_duration = "30m"
 
 [services.webhook]
 name = "webhook"
diff --git a/doc/swagger.yaml b/doc/swagger.yaml
index 416727f..c087a65 100644
--- a/doc/swagger.yaml
+++ b/doc/swagger.yaml
@@ -693,7 +693,6 @@ definitions:
         description: ID of the credential manifest used for this request.
         type: string
     required:
-    - expiration
     - issuerId
     - issuerKid
     - manifestId
@@ -729,7 +728,6 @@ definitions:
           This is an output only field.
         type: string
     required:
-    - expiration
     - issuerId
     - issuerKid
     - presentationDefinitionId
@@ -1194,7 +1192,7 @@ definitions:
       expiration:
         description: |-
           Expiration as defined in https://www.rfc-editor.org/rfc/rfc7519.html#section-4.1.4
-          Optional. When not specified, the request will be valid for a default duration.
+          Optional.
         type: string
       issuerId:
         description: |-
@@ -1262,7 +1260,7 @@ definitions:
       expiration:
         description: |-
           Expiration as defined in https://www.rfc-editor.org/rfc/rfc7519.html#section-4.1.4
-          Optional. When not specified, the request will be valid for a default duration.
+          Optional.
         type: string
       issuerId:
         description: |-
diff --git a/pkg/server/router/manifest.go b/pkg/server/router/manifest.go
index ff34971..92dc010 100644
--- a/pkg/server/router/manifest.go
+++ b/pkg/server/router/manifest.go
@@ -647,7 +647,7 @@ func (mr ManifestRouter) CreateRequest(c *gin.Context) {
 }
 
 func (mr ManifestRouter) serviceRequestFromRequest(request CreateManifestRequestRequest) (*model.Request, error) {
-	req, err := commonRequestToServiceRequest(request.CommonCreateRequestRequest, mr.service.Config().ExpirationDuration)
+	req, err := commonRequestToServiceRequest(request.CommonCreateRequestRequest)
 	if err != nil {
 		return nil, err
 	}
@@ -746,23 +746,18 @@ func (mr ManifestRouter) DeleteRequest(c *gin.Context) {
 	framework.Respond(c, nil, http.StatusNoContent)
 }
 
-func commonRequestToServiceRequest(request *CommonCreateRequestRequest, expirationDuration time.Duration) (*common.Request, error) {
-	var expiration time.Time
-	var err error
-
+func commonRequestToServiceRequest(request *CommonCreateRequestRequest) (*common.Request, error) {
+	req := &common.Request{
+		Audience:  request.Audience,
+		IssuerDID: request.IssuerDID,
+		IssuerKID: request.IssuerKID,
+	}
 	if request.Expiration != "" {
-		expiration, err = time.Parse(time.RFC3339, request.Expiration)
+		expiration, err := time.Parse(time.RFC3339, request.Expiration)
 		if err != nil {
 			return nil, err
 		}
-	} else {
-		expiration = time.Now().Add(expirationDuration)
-	}
-	req := &common.Request{
-		Audience:   request.Audience,
-		Expiration: expiration,
-		IssuerDID:  request.IssuerDID,
-		IssuerKID:  request.IssuerKID,
+		req.Expiration = &expiration
 	}
 	return req, nil
 }
diff --git a/pkg/server/router/model.go b/pkg/server/router/model.go
index 2781979..a0c8254 100644
--- a/pkg/server/router/model.go
+++ b/pkg/server/router/model.go
@@ -6,7 +6,7 @@ type CommonCreateRequestRequest struct {
 	Audience []string `json:"audience"`
 
 	// Expiration as defined in https://www.rfc-editor.org/rfc/rfc7519.html#section-4.1.4
-	// Optional. When not specified, the request will be valid for a default duration.
+	// Optional.
 	Expiration string `json:"expiration"`
 
 	// DID of the issuer of this presentation definition. The DID must have been previously created with the DID API,
diff --git a/pkg/server/router/presentation.go b/pkg/server/router/presentation.go
index 85219eb..b4c5ecc 100644
--- a/pkg/server/router/presentation.go
+++ b/pkg/server/router/presentation.go
@@ -532,7 +532,7 @@ func (pr PresentationRouter) CreateRequest(c *gin.Context) {
 }
 
 func (pr PresentationRouter) serviceRequestFromRequest(request CreateRequestRequest) (*model.Request, error) {
-	req, err := commonRequestToServiceRequest(request.CommonCreateRequestRequest, pr.service.Config().ExpirationDuration)
+	req, err := commonRequestToServiceRequest(request.CommonCreateRequestRequest)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/service/common/request.go b/pkg/service/common/request.go
index 6893636..651733f 100644
--- a/pkg/service/common/request.go
+++ b/pkg/service/common/request.go
@@ -19,7 +19,7 @@ type Request struct {
 	Audience []string `json:"audience,omitempty"`
 
 	// Expiration as defined in https://www.rfc-editor.org/rfc/rfc7519.html#section-4.1.4
-	Expiration time.Time `json:"expiration" validate:"required"`
+	Expiration *time.Time `json:"expiration,omitempty"`
 
 	// DID of the issuer of this presentation definition.
 	IssuerDID string `json:"issuerId" validate:"required"`
@@ -30,32 +30,38 @@ type Request struct {
 
 // ToServiceModel converts a storage model to a service model.
 func ToServiceModel(stored *StoredRequest) (*Request, error) {
-	expiration, err := time.Parse(time.RFC3339, stored.Expiration)
-	if err != nil {
-		return nil, errors.Wrap(err, "parsing expiration time")
+	request := &Request{
+		ID:        stored.ID,
+		Audience:  stored.Audience,
+		IssuerDID: stored.IssuerDID,
+		IssuerKID: stored.IssuerKID,
 	}
-
-	return &Request{
-		ID:         stored.ID,
-		Audience:   stored.Audience,
-		Expiration: expiration,
-		IssuerDID:  stored.IssuerDID,
-		IssuerKID:  stored.IssuerKID,
-	}, nil
+	if stored.Expiration != "" {
+		expiration, err := time.Parse(time.RFC3339, stored.Expiration)
+		if err != nil {
+			return nil, errors.Wrap(err, "parsing expiration time")
+		}
+		request.Expiration = &expiration
+	}
+	return request, nil
 }
 
 // CreateStoredRequest creates a StoredRequest with the associated signed JWT populated. In addition to the fields
 // present in request, the JWT will also include a claim with claimName and claimValue.
 func CreateStoredRequest(ctx context.Context, keyStore *keystore.Service, claimName string, claimValue any, request Request, id string) (*StoredRequest, error) {
 	requestID := uuid.NewString()
-	token, err := jwt.NewBuilder().
+	builder := jwt.NewBuilder().
 		Claim(claimName, claimValue).
 		Audience(request.Audience).
-		Expiration(request.Expiration).
 		Issuer(request.IssuerDID).
 		NotBefore(time.Now()).
-		JwtID(requestID).
-		Build()
+		JwtID(requestID)
+	var expirationString string
+	if request.Expiration != nil {
+		builder.Expiration(*request.Expiration)
+		expirationString = request.Expiration.Format(time.RFC3339)
+	}
+	token, err := builder.Build()
 	if err != nil {
 		return nil, errors.Wrap(err, "building jwt")
 	}
@@ -67,7 +73,7 @@ func CreateStoredRequest(ctx context.Context, keyStore *keystore.Service, claimN
 	stored := &StoredRequest{
 		ID:          requestID,
 		Audience:    request.Audience,
-		Expiration:  request.Expiration.Format(time.RFC3339),
+		Expiration:  expirationString,
 		IssuerDID:   request.IssuerDID,
 		IssuerKID:   request.IssuerKID,
 		ReferenceID: id,


Test patch:
diff --git a/config/test.toml b/config/test.toml
index 302854f..65f1785 100644
--- a/config/test.toml
+++ b/config/test.toml
@@ -69,7 +69,6 @@ name = "manifest"
 
 [services.presentation]
 name = "presentation"
-expiration_duration = "30m"
 
 [services.webhook]
 name = "webhook"
diff --git a/pkg/server/router/manifest_test.go b/pkg/server/router/manifest_test.go
index 61492cc..6cb4c8f 100644
--- a/pkg/server/router/manifest_test.go
+++ b/pkg/server/router/manifest_test.go
@@ -3,7 +3,6 @@ package router
 import (
 	"context"
 	"testing"
-	"time"
 
 	"github.com/TBD54566975/ssi-sdk/credential/exchange"
 	manifestsdk "github.com/TBD54566975/ssi-sdk/credential/manifest"
@@ -238,7 +237,7 @@ func getValidManifestRequestRequest(issuerDID *did.CreateDIDResponse, kid string
 				Audience:   []string{"mario"},
 				IssuerDID:  issuerDID.DID.ID,
 				IssuerKID:  kid,
-				Expiration: time.Now().Add(100 * time.Second),
+				Expiration: &In100Seconds,
 			},
 			ManifestID: createdManifest.Manifest.ID,
 		},
diff --git a/pkg/server/router/presentation_test.go b/pkg/server/router/presentation_test.go
index 43d0b7e..9b45fdf 100644
--- a/pkg/server/router/presentation_test.go
+++ b/pkg/server/router/presentation_test.go
@@ -39,6 +39,12 @@ func TestPresentationDefinitionRouter(t *testing.T) {
 	})
 }
 
+var (
+	SampleExpirationTime = time.Date(2023, 10, 10, 10, 10, 10, 0, time.UTC)
+	In30Seconds          = time.Now().Add(30 * time.Second)
+	In100Seconds         = time.Now().Add(100 * time.Second)
+)
+
 func TestPresentationDefinitionService(t *testing.T) {
 	for _, test := range testutil.TestDatabases {
 		t.Run(test.Name, func(t *testing.T) {
@@ -116,7 +122,7 @@ func TestPresentationDefinitionService(t *testing.T) {
 						Audience:   []string{"did:web:heman"},
 						IssuerDID:  authorDID.DID.ID,
 						IssuerKID:  authorDID.DID.VerificationMethod[0].ID,
-						Expiration: time.Date(2023, 10, 10, 10, 10, 10, 0, time.UTC),
+						Expiration: &SampleExpirationTime,
 					},
 					PresentationDefinitionID: pd.ID,
 				}
@@ -152,7 +158,7 @@ func TestPresentationDefinitionService(t *testing.T) {
 							Audience:   []string{"did:web:heman"},
 							IssuerDID:  authorDID.DID.ID,
 							IssuerKID:  authorDID.DID.VerificationMethod[0].ID,
-							Expiration: time.Now().Add(30 * time.Second),
+							Expiration: &In30Seconds,
 						},
 						PresentationDefinitionID: pd.ID,
 					},
@@ -175,7 +181,7 @@ func TestPresentationDefinitionService(t *testing.T) {
 						Request: common.Request{
 							IssuerDID:  authorDID.DID.ID,
 							IssuerKID:  authorDID.DID.VerificationMethod[0].ID,
-							Expiration: time.Now().Add(30 * time.Second),
+							Expiration: &In30Seconds,
 						},
 						PresentationDefinitionID: pd.ID,
 					},