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

GetClaim() returning interface is awkward to use. Use output parameter instead? #182

Closed
arianvp opened this issue May 18, 2022 · 3 comments
Assignees
Labels
backend enhancement New feature or request sdk
Milestone

Comments

@arianvp
Copy link

arianvp commented May 18, 2022

Is your feature request related to a problem? Please describe.

The following code is awkward as I need to guess that the structure that GetClaim() returns is map[string]string.

		val, ok := introspectionResponse.GetClaim("cnf").(map[string]string)
		if !ok {
			http.Error(rw, "", http.StatusUnauthorized)
			return
		}

		expectedClientCertificateHash, err := base64.URLEncoding.DecodeString(val["x5t#S256"])
		if err != nil {
			http.Error(rw, "", http.StatusUnauthorized)
			return
		}
		if !bytes.Equal(clientCertificateHash[:], expectedClientCertificateHash) {
			http.Error(rw, "", http.StatusUnauthorized)
			return
		}

Describe the solution you'd like

I'd prefer GetClaim() to take an output parameter instead. so that I can marshal safely to my own struct type:

func (r IntrospectionResponse) GetClaim(key string, out interface{}) error {...}
type ConfirmationClaim struct {
        CertificateThumbprint string `json:"x5t#S256"`
}

var cnf ConfirmationClaim
err := introspectionResponse.GetClaim("cnf", &cnf)
if err != nil { ... }
expectedClientCertificateHash, err := base64.URLEncoding.DecodeString(cnf.CertificateThumbprint)
if err != nil { ... }
if !bytes.Equal(clientCertificateHash[:], cnf.CertificateThumbprint {
	http.Error(rw, "", http.StatusUnauthorized)
	return
}

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Add any other context or screenshots about the feature request here.

@arianvp arianvp added the enhancement New feature or request label May 18, 2022
@fforootd
Copy link
Member

Hi @arianvp - I agree that this can be improved 😁

Currently all hands are in the version 2 prepwork for ZITADEL, so it might take 2-4 weeks before someone can get a hold of this issue. Hope thats not too bad for you.

@arianvp
Copy link
Author

arianvp commented May 19, 2022

No rush. The code works as is. It's just awkward.

@muhlemmer
Copy link
Collaborator

#283 should improve this. If you have any other feedback, please let us know

@github-project-automation github-project-automation bot moved this from 📨 Product Backlog to ✅ Done in Product Management Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend enhancement New feature or request sdk
Projects
None yet
Development

No branches or pull requests

4 participants