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

ensure only one key is deleted if multiple with same name #862

Merged
merged 4 commits into from
Sep 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions backend/authschemes/webauthn/dtos.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ type ListCredentialsOutput struct {
type CredentialEntry struct {
CredentialName string `json:"credentialName"`
DateCreated time.Time `json:"dateCreated"`
CredentialID string `json:"credentialId"`
}
25 changes: 20 additions & 5 deletions backend/authschemes/webauthn/webauthn.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
package webauthn

import (
"bytes"
"encoding/hex"
"encoding/json"
"errors"
"net"
Expand Down Expand Up @@ -233,14 +235,15 @@ func (a WebAuthn) BindRoutes(r chi.Router, bridge authschemes.AShirtAuthBridge)
return a.getCredentials(callingUserID, bridge)
}))

remux.Route(r, "DELETE", "/credential/{credentialName}", remux.JSONHandler(func(r *http.Request) (interface{}, error) {
remux.Route(r, "DELETE", "/credential/{credentialID}", remux.JSONHandler(func(r *http.Request) (interface{}, error) {
callingUserID := middleware.UserID(r.Context())
dr := remux.DissectJSONRequest(r)
credentialName := dr.FromURL("credentialName").Required().AsString()
credentialID := dr.FromURL("credentialID").Required().AsString()
if dr.Error != nil {
return nil, dr.Error
}
return nil, a.deleteCredential(callingUserID, credentialName, bridge)
credIDByteArr, _ := hex.DecodeString(credentialID)
return nil, a.deleteCredential(callingUserID, credIDByteArr, bridge)
}))

remux.Route(r, "PUT", "/credential", remux.JSONHandler(func(r *http.Request) (interface{}, error) {
Expand Down Expand Up @@ -324,13 +327,14 @@ func (a WebAuthn) getCredentials(userID int64, bridge authschemes.AShirtAuthBrid
return CredentialEntry{
CredentialName: cred.CredentialName,
DateCreated: cred.CredentialCreatedDate,
CredentialID: hex.EncodeToString(cred.ID),
}
})
output := ListCredentialsOutput{results}
return &output, nil
}

func (a WebAuthn) deleteCredential(userID int64, credentialName string, bridge authschemes.AShirtAuthBridge) error {
func (a WebAuthn) deleteCredential(userID int64, credentialID []byte, bridge authschemes.AShirtAuthBridge) error {
auth, err := bridge.FindUserAuthByUserID(userID)
if err != nil {
return backend.WrapError("Unable to find user", err)
Expand All @@ -343,7 +347,7 @@ func (a WebAuthn) deleteCredential(userID int64, credentialName string, bridge a
}

results := helpers.Filter(creds, func(cred AShirtWebauthnCredential) bool {
return cred.CredentialName != credentialName
return !bytes.Equal(cred.ID, credentialID)
})
encodedCreds, err := json.Marshal(results)
if err != nil {
Expand Down Expand Up @@ -396,6 +400,17 @@ func (a WebAuthn) beginRegistration(w http.ResponseWriter, r *http.Request, brid
user = makeAddCredentialWebAuthnUser(info.UserID, info.Username, info.CredentialName, info.ExistingCredentials)
}

idx, _ := helpers.Find(info.ExistingCredentials, func(cred AShirtWebauthnCredential) bool {
return strings.ToLower(cred.CredentialName) == strings.ToLower(info.CredentialName)
})

if idx != -1 {
return nil, backend.BadInputErr(
errors.New("user trying to register with taken credential name"),
"Credential name is already taken",
)
}

credExcludeList := make([]protocol.CredentialDescriptor, len(user.Credentials))
for i, cred := range user.Credentials {
credExcludeList[i] = protocol.CredentialDescriptor{
Expand Down
2 changes: 1 addition & 1 deletion backend/helpers/functional.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func Map[T any, U any](slice []T, mapFn func(T) U) []U {
return result
}

// Find is a generic function that searches through a list searching for all items that match
// Filter is a generic function that searches through a list searching for all items that match
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oops!

// the given predicate. This returns the elements that matched the predicate in the order they were
// encountered.
func Filter[T any](slice []T, predicate func(T) bool) []T {
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/authschemes/webauthn/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ export async function listWebauthnCredentials(): Promise<CredentialList> {

}

export async function deleteWebauthnCredential(i: { credentialName: string }): Promise<CredentialList> {
return await req('DELETE', `/auth/webauthn/credential/${i.credentialName}`)
export async function deleteWebauthnCredential(i: { credentialId: string }): Promise<CredentialList> {
return await req('DELETE', `/auth/webauthn/credential/${i.credentialId}`)
}

export async function modifyCredentialName(i: { credentialName: string, newCredentialName: string }): Promise<void> {
Expand Down
9 changes: 5 additions & 4 deletions frontend/src/authschemes/webauthn/settings/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const CredentialList = (props: {
return () => { props.offReload(wiredCredentials.reload) }
})

const deleteModal = useModal<{ credentialName: string }>(mProps => <DeleteCredentialModal {...mProps} />, wiredCredentials.reload)
const deleteModal = useModal<{ credentialId: string, credentialName: string }>(mProps => <DeleteCredentialModal {...mProps} />, wiredCredentials.reload)
const modifyModal = useModal<{ credentialName: string }>(mProps => <EditCredentialModal {...mProps} />, wiredCredentials.reload)

return (<>
Expand All @@ -53,7 +53,7 @@ const CredentialList = (props: {
<div>
<Table columns={['Credential Name', 'Date Created', 'Actions']}>
{data.credentials.map(credentialEntry => {
const { credentialName, dateCreated } = credentialEntry
const { credentialName, dateCreated, credentialId } = credentialEntry
return (
<tr key={credentialName}>
<td>{credentialName}</td>
Expand All @@ -64,7 +64,7 @@ const CredentialList = (props: {
modifyModal.show({ credentialName })
}}>Edit</Button>
<Button danger small onClick={() => {
deleteModal.show({ credentialName })
deleteModal.show({ credentialId, credentialName })
}}>Delete</Button>
</ButtonGroup>
</td>
Expand Down Expand Up @@ -146,14 +146,15 @@ const AddCredentialModal = (props: {

const DeleteCredentialModal = (props: {
credentialName: string,
credentialId: string,
onRequestClose: () => void,
}) => (
<ChallengeModalForm
modalTitle="Delete Credential"
warningText="Are you sure you want to delete this security credential?"
submitText="Delete"
challengeText={props.credentialName}
handleSubmit={() => deleteWebauthnCredential({ credentialName: props.credentialName })}
handleSubmit={() => deleteWebauthnCredential({ credentialId: props.credentialId })}
onRequestClose={props.onRequestClose}
/>
)
Expand Down
1 change: 1 addition & 0 deletions frontend/src/authschemes/webauthn/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,5 @@ export type CredentialList = {
export type CredentialEntry = {
credentialName: string
dateCreated: Date
credentialId: string
}