OpenCHAMI / smd

MIT License
0 stars 4 forks source link

Added OpenCHAMI middleware to router.NewRouter() #29

Closed davidallendj closed 2 months ago

davidallendj commented 2 months ago

Addresses #27 by adding the OpenCHAMI middleware.

davidallendj commented 2 months ago

Seems that the OpenCHAMI JWTAuth used with AuthenticatorWithRequiredClaims is not compatible with the one being used with github.com/OpenCHAMI/jwtauth/v5 and the PR that adds the jwt.NewKeySet and JWKS functionality has not be merged upstream yet (https://github.com/go-chi/jwtauth/pull/85).

synackd commented 2 months ago

Is the functionality in that upstream PR able to be put into the OpenCHAMI jwtauth for now?

davidallendj commented 2 months ago

Is the functionality in that upstream PR able to be put into the OpenCHAMI jwtauth for now?

I think the OpenCHAMI jwtauth already has that functionality. I think we would just have to change the import in the OpenCHAMI middleware to point to the OpenCHAMI jwtauth for now and then rebase it with the upstream go-chi changes. Once it finally gets merged, we can change the import back to upstream and then delete the OpenCHAMI one.

synackd commented 2 months ago

OK. Is this ready to be tested?

davidallendj commented 2 months ago

OK. Is this ready to be tested?

Looks like SMD is ready, but we'll have to switch the authenticator to use the OpenCHAMI jwtauth for now.

synackd commented 2 months ago

I'm getting this error when building:

# github.com/OpenCHAMI/smd/v2/cmd/smd
cmd/smd/routers.go:72:61: cannot use s.tokenAuth (variable of type *"github.com/OpenCHAMI/jwtauth/v5".JWTAuth) as *"github.com/go-chi/jwtauth/v5".JWTAuth value in argument to openchami_authenticator.AuthenticatorWithRequiredClaims
cmd/smd/auth.go:13:2: "github.com/lestrrat-go/jwx/jwt" imported and not used
make: *** [Makefile:55: smd] Error 1
synackd commented 2 months ago

Looks like switching the import to use the OpenCHAMI jwtauth will solve this as discussed here.

synackd commented 2 months ago

SMD is now building with those changes.