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

Support customization of action and object getter #65

Closed
wants to merge 1 commit into from

Conversation

suzaku
Copy link

@suzaku suzaku commented Dec 7, 2021

Sometimes the Casbin policies don't match perfectly with the HTTP request paths and methods, this middleware could be more useful if it allows the customization of these two entities.

@hsluoyz
Copy link
Contributor

hsluoyz commented Dec 7, 2021

@appleboy @Abingcbc plz review

@aldas
Copy link
Contributor

aldas commented Dec 7, 2021

I have created #66 instead of this solution. It will be more generic as it allows to choose enforcers.

Actions/Methods can be changed before Enforce call like that:

ce, _ := casbin.NewEnforcer("auth_model.conf", "auth_policy.csv")
cnf := Config{
	EnforceHandler: func(c echo.Context, user string) (bool, error) {
		method := c.Request().Method
		if strings.HasPrefix(c.Request().URL.Path, "/user/bob") {
			method += "_SELF"
		}
		return ce.Enforce(user, c.Request().URL.Path, method)
	},
}

@suzaku
Copy link
Author

suzaku commented Dec 7, 2021

I have created #66 instead of this solution. It will be more generic as it allows to choose enforcers.

Actions/Methods can be changed before Enforce call like that:

ce, _ := casbin.NewEnforcer("auth_model.conf", "auth_policy.csv")
cnf := Config{
	EnforceHandler: func(c echo.Context, user string) (bool, error) {
		method := c.Request().Method
		if strings.HasPrefix(c.Request().URL.Path, "/user/bob") {
			method += "_SELF"
		}
		return ce.Enforce(user, c.Request().URL.Path, method)
	},
}

Cool, very flexible. I have 2 questions though.

  1. Will it confuse users if they custom the user getter with a specialize field but have to use EnforceHandler for the rest?
  2. If I need to write a EnforceHandler just to override one field, I might just as well write the whole middleware in my project instead of depending on an external library, right?

@aldas
Copy link
Contributor

aldas commented Dec 7, 2021

  1. User getter is part of public API and can not be changed/removed.
  2. "If I need to write a EnforceHandler just to override one field" then why create ActionGetter at all?

compare ActionGetter

		ActionGetter: func(c echo.Context) string {
			method := c.Request().Method
			if strings.HasPrefix(c.Request().URL.Path, "/user/bob") {
				method += "_SELF"
			}
			return method
		},

with EnforceHandler

		EnforceHandler: func(c echo.Context, user string) (bool, error) {
			method := c.Request().Method
			if strings.HasPrefix(c.Request().URL.Path, "/user/bob") {
				method += "_SELF"
			}
			return ce.Enforce(user, c.Request().URL.Path, method)
		},

pretty much the same but latter allows things like #58 need

If you need completely custom implementation. Then this would be bare minimum

	e := echo.New()
	e.Use(func(next echo.HandlerFunc) echo.HandlerFunc {
		ce, _ := casbin.NewEnforcer("auth_model.conf", "auth_policy.csv")
		return func(c echo.Context) error {
			username, _, _ := c.Request().BasicAuth()
			ok, err := ce.Enforce(username, c.Request().URL.Path, c.Request().Method)
			if err != nil {
				return err
			}
			if !ok {
				return echo.ErrForbidden
			}
			return next(c)
		}
	})

@suzaku
Copy link
Author

suzaku commented Dec 8, 2021

Yes, the real problem is that this middleware is indeed very simple, so instead of making it customizable, maybe one should just implement it in whatever way suitable in their own project. Thanks for your clarification.

@suzaku suzaku closed this Dec 8, 2021
@suzaku suzaku deleted the custom-obj-act branch December 8, 2021 00:02
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.

3 participants