should the middleware be permissive?

[idc77]

Hello again,

I'm not sure what vision you had with this package, I, for one, am trying to avoid running a separate
https://github.com/gogatekeeper/gatekeeper
in front of every http api I create.
I stumbled upon this package because I wanted to write my own middleware and thought why re-invent the wheel.

Usually (or it's one possibility) GETting something does not require someone to be logged in, to attract visitors.
Only modifying requests should require authentication.
Of course I can achieve that by not putting the middleware in front of every handler that requires it, or rather...
putting the middleware in front of every handler that requires authentication and not putting it before GET requests.

This might or might not be an issue for you.

But why would one still do it? Convenience.
When you're able to just say something like this:

	h := handler.New(client)

	v1 := r.Group("/api/v1")
	v1.Use(oidcHandler)
	v1.GET("/oidc", h.OIDCClaimsHandler) // demo

	// Entity Routes

	post := v1.Group("/post")
	comment := v1.Group("/comment")
	h.PostRoutes(post)
	h.CommentRoutes(comment)

vs passing the oidcHandler to every single route, which I'm going to do now.

~

[simongottschlag]

Hi again @idc77!

The main idea of the package is to easily enable OpenID Connect for APIs. 👍

My opinion in the matter of hand is that the middleware shouldn't be aware of the actual routes or methods, but leave it up to the developer to define how it should be used.

In the gin example, something like this is what I would do for an API where all routes doesn't require authentication:
https://github.com/gin-gonic/gin#using-middleware

Does it make sense?

[idc77]

Yes but,

	oidcHandler := oidcgin.New(
		options.WithIssuer(cfg.Issuer),
		options.WithRequiredTokenType("JWT"),
		options.WithRequiredAudience(cfg.Audience),
	)

        // ...

	post := v1.Group("/post")
	{
		post.GET("/", h.PostList)
		post.GET("/:id", h.PostOne)
		post.POST("/", oidcHandler, h.PostCreate)
		post.PUT("/:id", oidcHandler, h.PostUpdateOne)
		post.DELETE("/:id", oidcHandler, h.PostDelete)
	}

The GETs here are without a route guard, but every route that has a guard has to be explicitly stated.

In the initial post I do what you linked. All routes are guarded via .Use(oidcHandler).

The question is, in order to save some explicit stating of the oidcHandler for every.single.route, should the middleware not fail if a required criteria (set by options.WithRequiredTokenType for instance).
The current behavior is it fails or returns an error if the required criteria aren't met.

If that's the way you'd like it to work then fine and close the issue.

The alternative is to use the middleware in the way you linked, only GET requests are not "guarded".
Example:

	oidcHandler := oidcgin.New(
                options.BypassGETRequests(), // <- new convenience option for GET requests
		options.WithIssuer(cfg.Issuer),
		options.WithRequiredTokenType("JWT"),
		options.WithRequiredAudience(cfg.Audience),
	)

This would do something like

if c.Request.Method == http.MethodGET {
  c.Next()
}

So the question is, are you fine with explicitly guarding every single route,
or would an option to ignore GET requests, since they have no payload, and thus being able to use
authorized.Use(AuthRequired()) from your linked example.

I'm proposing the options.BypassGETRequests() or call it WhiteListGetRequests or w/e to be added which would exempt GET requests from failing if no token is present. A convenience feature.

[simongottschlag]

Hi!

I understand what you want. 👍 I don't see that as something the middleware should know about.

You should be able to wrap this middleware in your own (just a few lines most likely) and be able to skip it when you think it's suitable.

I'll go ahead and close the issue as you wrote, but please post your code if you think anybody else might have the same need in the future.

Thanks for opening the issue and I hope the middleware works for you! 😊