Skip to content

fix(codegen): dedupe security schemes by type #3690

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

Merged
merged 2 commits into from
May 12, 2025

Conversation

disintegrator
Copy link
Contributor

@disintegrator disintegrator commented Apr 2, 2025

This commit solves a bug where designs that use multiple security schemes of the same type break the generated Auther interface for a service as well as endpoint-related init methods due to unnecessary code duplication from ranging over .Schemes.

Reproducer

import (
	. "goa.design/goa/v3/dsl"
)

var AcmeAPIKey = APIKeySecurity("acme_api_key")
var AcmeTenant = APIKeySecurity("acme_tenant_id")

var _ = Service("customers", func() {
	Security(AcmeAPIKey, AcmeTenant)

        // ...
})

This currently results in code like this:

// Auther defines the authorization functions to be implemented by the service.
type Auther interface {
	// APIKeyAuth implements the authorization logic for the APIKey security scheme.
	APIKeyAuth(ctx context.Context, key string, schema *security.APIKeyScheme) (context.Context, error)
	// APIKeyAuth implements the authorization logic for the APIKey security scheme.
	APIKeyAuth(ctx context.Context, key string, schema *security.APIKeyScheme) (context.Context, error)
}

And:

// NewEndpoints wraps the methods of the "assets" service with endpoints.
func NewEndpoints(s Service) *Endpoints {
	// Casting service to Auther interface
	a := s.(Auther)
	return &Endpoints{
		UploadOpenAPIv3: NewUpdateDetailsEndpoint(s, a.APIKeyAuth, a.APIKeyAuth),
		//                                                         ^ 🚨
	}
}

// ...

func NewUpdateDetailsEndpoint(s Service, authAPIKeyFn security.AuthAPIKeyFunc, authAPIKeyFn security.AuthAPIKeyFunc) goa.Endpoint {
//                                                                             ^ 🚨
	// ...
}

This commit solves a bug where designs that use multiple security
schemes _of the same type_ break the generated `Auther` interface for a
service as well as endpoint-related init methods due to unnecessary code
duplication from ranging over `.Schemes`.
@disintegrator
Copy link
Contributor Author

Just need to figure out testing but putting it up PR to start discussion.

@raphael
Copy link
Member

raphael commented Apr 7, 2025

Thank you for the PR, it looks great. Will be happy to merge once there's a test for it.

@raphael
Copy link
Member

raphael commented Apr 27, 2025

This is a good bug fix that I'd like to have in the next release of Goa, let me know if I can help with the tests.

@disintegrator
Copy link
Contributor Author

@raphael sorry for the delay, i've been a little swamped. I'll set some time aside for writing tests this week.

@disintegrator
Copy link
Contributor Author

disintegrator commented May 11, 2025

Hey @raphael, added a couple of test DSLs to the mix that should reproduce this bug and confirm it's fixed by this PR.

@raphael
Copy link
Member

raphael commented May 11, 2025

Awesome, thank you

@raphael raphael merged commit feb52b3 into goadesign:v3 May 12, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants