Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(protocol): change *http.Request parameter #246

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 45 additions & 24 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,34 +101,45 @@ func main() {
```go
package example

import "io"

func BeginRegistration(w http.ResponseWriter, r *http.Request) {
user := datastore.GetUser() // Find or create the new user
options, session, err := webAuthn.BeginRegistration(user)
// handle errors if present
// store the sessionData values
JSONResponse(w, options, http.StatusOK) // return the options generated
// options.publicKey contain our registration options
user := datastore.GetUser() // Find or create the new user
options, session, err := webAuthn.BeginRegistration(user)
// handle errors if present
// store the sessionData values
JSONResponse(w, options, http.StatusOK) // return the options generated
// options.publicKey contain our registration options
}

func FinishRegistration(w http.ResponseWriter, r *http.Request) {
user := datastore.GetUser() // Get the user

// Get the session data stored from the function above
session := datastore.GetSession()

credential, err := webAuthn.FinishRegistration(user, session, r)
if err != nil {
// Handle Error and return.
user := datastore.GetUser() // Get the user

return
}

// If creation was successful, store the credential object
// Pseudocode to add the user credential.
user.AddCredential(credential)
datastore.SaveUser(user)
// Get the session data stored from the function above
session := datastore.GetSession()

JSONResponse(w, "Registration Success", http.StatusOK) // Handle next steps
body, err := io.ReadAll(r.Body)
if err != nil{
// Handle Error and return.

return
}

defer body.Close()

credential, err := webAuthn.FinishRegistration(user, session, body)
if err != nil {
// Handle Error and return.

return
}

// If creation was successful, store the credential object
// Pseudocode to add the user credential.
user.AddCredential(credential)
datastore.SaveUser(user)

JSONResponse(w, "Registration Success", http.StatusOK) // Handle next steps
}
```

Expand Down Expand Up @@ -159,8 +170,17 @@ func FinishLogin(w http.ResponseWriter, r *http.Request) {

// Get the session data stored from the function above
session := datastore.GetSession()

credential, err := webAuthn.FinishLogin(user, session, r)

body, err := io.ReadAll(r.Body)
if err != nil{
// Handle Error and return.

return
}

defer body.Close()

credential, err := webAuthn.FinishLogin(user, session, body)
if err != nil {
// Handle Error and return.

Expand All @@ -176,6 +196,7 @@ func FinishLogin(w http.ResponseWriter, r *http.Request) {

JSONResponse(w, "Login Success", http.StatusOK)
}

```

## Modifying Credential Options
Expand Down
15 changes: 5 additions & 10 deletions protocol/assertion.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import (
"encoding/base64"
"encoding/json"
"fmt"
"io"
"net/http"

"github.com/go-webauthn/webauthn/protocol/webauthncose"
)
Expand Down Expand Up @@ -49,24 +47,21 @@ type ParsedAssertionResponse struct {
// ParseCredentialRequestResponse parses the credential request response into a format that is either required by the
// specification or makes the assertion verification steps easier to complete. This takes a http.Request that contains
// the assertion response data in a raw, mostly base64 encoded format, and parses the data into manageable structures.
func ParseCredentialRequestResponse(response *http.Request) (*ParsedCredentialAssertionData, error) {
if response == nil || response.Body == nil {
func ParseCredentialRequestResponse(credentialResponse []byte) (*ParsedCredentialAssertionData, error) {
if credentialResponse == nil {
return nil, ErrBadRequest.WithDetails("No response given")
}

defer response.Body.Close()
defer io.Copy(io.Discard, response.Body)

return ParseCredentialRequestResponseBody(response.Body)
return ParseCredentialRequestResponseBody(credentialResponse)
}

// ParseCredentialRequestResponseBody parses the credential request response into a format that is either required by
// the specification or makes the assertion verification steps easier to complete. This takes an io.Reader that contains
// the assertion response data in a raw, mostly base64 encoded format, and parses the data into manageable structures.
func ParseCredentialRequestResponseBody(body io.Reader) (par *ParsedCredentialAssertionData, err error) {
func ParseCredentialRequestResponseBody(credentialResponse []byte) (par *ParsedCredentialAssertionData, err error) {
var car CredentialAssertionResponse

if err = decodeBody(body, &car); err != nil {
if err = json.Unmarshal(credentialResponse, &car); err != nil {
Comment on lines +61 to +64
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using predefined constants for error details to maintain consistency.

The error handling code in ParseCredentialRequestResponseBody uses a hardcoded string for the error detail. Using a predefined constant would enhance maintainability and consistency across the codebase.

- return nil, ErrBadRequest.WithDetails("Parse error for Assertion").WithInfo(err.Error())
+ return nil, ErrBadRequest.WithDetails(ErrParseAssertion).WithInfo(err.Error())

Committable suggestion was skipped due to low confidence.

return nil, ErrBadRequest.WithDetails("Parse error for Assertion").WithInfo(err.Error())
}

Expand Down
15 changes: 1 addition & 14 deletions protocol/assertion_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
package protocol

import (
"bytes"
"encoding/base64"
"io"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -94,24 +92,13 @@ func TestParseCredentialRequestResponse(t *testing.T) {
},
errString: "",
},
{
name: "ShouldHandleTrailingData",
args: args{
"trailingData",
},
expected: nil,
errString: "Parse error for Assertion",
errType: "invalid_request",
errDetails: "Parse error for Assertion",
errInfo: "The body contains trailing data",
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

body := io.NopCloser(bytes.NewReader([]byte(testAssertionResponses[tc.args.responseName])))
body := []byte(testAssertionResponses[tc.args.responseName])

actual, err := ParseCredentialRequestResponseBody(body)

Expand Down
18 changes: 7 additions & 11 deletions protocol/credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ package protocol
import (
"crypto/sha256"
"encoding/base64"
"io"
"net/http"
"encoding/json"
)

// Credential is the basic credential type from the Credential Management specification that is inherited by WebAuthn's
Expand Down Expand Up @@ -60,23 +59,20 @@ type ParsedCredentialCreationData struct {

// ParseCredentialCreationResponse is a non-agnostic function for parsing a registration response from the http library
// from stdlib. It handles some standard cleanup operations.
func ParseCredentialCreationResponse(response *http.Request) (*ParsedCredentialCreationData, error) {
if response == nil || response.Body == nil {
func ParseCredentialCreationResponse(creationResponse []byte) (*ParsedCredentialCreationData, error) {
if creationResponse == nil {
return nil, ErrBadRequest.WithDetails("No response given")
}

defer response.Body.Close()
defer io.Copy(io.Discard, response.Body)

return ParseCredentialCreationResponseBody(response.Body)
return ParseCredentialCreationResponseBody(creationResponse)
}

// ParseCredentialCreationResponseBody is an agnostic version of ParseCredentialCreationResponse. Implementers are
// therefore responsible for managing cleanup.
func ParseCredentialCreationResponseBody(body io.Reader) (pcc *ParsedCredentialCreationData, err error) {
func ParseCredentialCreationResponseBody(creationResponse []byte) (pcc *ParsedCredentialCreationData, err error) {
var ccr CredentialCreationResponse

if err = decodeBody(body, &ccr); err != nil {
if err = json.Unmarshal(creationResponse, &ccr); err != nil {
return nil, ErrBadRequest.WithDetails("Parse error for Registration").WithInfo(err.Error())
}

Expand Down Expand Up @@ -204,7 +200,7 @@ func (pcc *ParsedCredentialCreationData) Verify(storedChallenge string, verifyUs
// 9. Return the appid extension value from the Session data.
func (ppkc ParsedPublicKeyCredential) GetAppID(authExt AuthenticationExtensions, credentialAttestationType string) (appID string, err error) {
var (
value, clientValue interface{}
value, clientValue any
enableAppID, ok bool
)

Expand Down
15 changes: 1 addition & 14 deletions protocol/credential_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
package protocol

import (
"bytes"
"encoding/base64"
"io"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -94,22 +92,11 @@ func TestParseCredentialCreationResponse(t *testing.T) {
},
errString: "",
},
{
name: "ShouldHandleTrailingData",
args: args{
responseName: "trailingData",
},
expected: nil,
errString: "Parse error for Registration",
errType: "invalid_request",
errDetails: "Parse error for Registration",
errInfo: "The body contains trailing data",
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
body := io.NopCloser(bytes.NewReader([]byte(testCredentialRequestResponses[tc.args.responseName])))
body := []byte(testCredentialRequestResponses[tc.args.responseName])

actual, err := ParseCredentialCreationResponseBody(body)

Expand Down
23 changes: 0 additions & 23 deletions protocol/decoder.go

This file was deleted.

42 changes: 38 additions & 4 deletions webauthn/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package webauthn
import (
"bytes"
"fmt"
"io"
"net/http"
"net/url"
"time"
Expand Down Expand Up @@ -162,20 +163,53 @@ func WithLoginRelyingPartyID(id string) LoginOption {
}

// FinishLogin takes the response from the client and validate it against the user credentials and stored session data.
func (webauthn *WebAuthn) FinishLogin(user User, session SessionData, response *http.Request) (*Credential, error) {
parsedResponse, err := protocol.ParseCredentialRequestResponse(response)
func (webauthn *WebAuthn) FinishLogin(user User, session SessionData, clientResponse any) (*Credential, error) {
body, err := webauthn.processResponse(clientResponse)
if err != nil {
return nil, err
}

parsedResponse, err := protocol.ParseCredentialRequestResponse(body)
if err != nil {
return nil, err
}

return webauthn.ValidateLogin(user, session, parsedResponse)
}

func (webauthn *WebAuthn) processResponse(data any) ([]byte, error) {
var (
body []byte
err error
)

switch cl := data.(type) {
case *http.Request:
body, err = io.ReadAll(cl.Body)
_ = cl.Body.Close()

case io.Reader:
body, err = io.ReadAll(cl)

case []byte:
body = cl

default:
return nil, protocol.ErrBadRequest.WithDetails("Invalid client response type")
}

if err != nil {
return nil, err
}

return body, nil
}

// FinishDiscoverableLogin takes the response from the client and validate it against the handler and stored session data.
// The handler helps to find out which user must be used to validate the response. This is a function defined in your
// business code that will retrieve the user from your persistent data.
func (webauthn *WebAuthn) FinishDiscoverableLogin(handler DiscoverableUserHandler, session SessionData, response *http.Request) (*Credential, error) {
parsedResponse, err := protocol.ParseCredentialRequestResponse(response)
func (webauthn *WebAuthn) FinishDiscoverableLogin(handler DiscoverableUserHandler, session SessionData, clientResponse []byte) (*Credential, error) {
parsedResponse, err := protocol.ParseCredentialRequestResponse(clientResponse)
if err != nil {
return nil, err
}
Expand Down
8 changes: 6 additions & 2 deletions webauthn/registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package webauthn
import (
"bytes"
"fmt"
"net/http"
"net/url"
"time"

Expand Down Expand Up @@ -203,7 +202,12 @@ func WithRegistrationRelyingPartyName(name string) RegistrationOption {

// FinishRegistration takes the response from the authenticator and client and verify the credential against the user's
// credentials and session data.
func (webauthn *WebAuthn) FinishRegistration(user User, session SessionData, response *http.Request) (*Credential, error) {
func (webauthn *WebAuthn) FinishRegistration(user User, session SessionData, authenticatorResponse any) (*Credential, error) {
response, err := webauthn.processResponse(authenticatorResponse)
if err != nil {
return nil, err
}

parsedResponse, err := protocol.ParseCredentialCreationResponse(response)
if err != nil {
return nil, err
Expand Down