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

Introduce RBAC checking via custom authHandler function #218

Open
wants to merge 28 commits into
base: v1.x/staging
Choose a base branch
from

Conversation

DivergentEuropeans
Copy link
Member

@DivergentEuropeans DivergentEuropeans commented May 24, 2021

PR (2 of 2)
PR 1: zowe/zss#281

Signed-off-by: Leanid Astrakou [email protected]

@lchudinov
Copy link
Contributor

lchudinov commented May 24, 2021

The code is questionable because it introduces a dependency on zss by calling functions defined there.

Could we introduce int registerHttpMiddleware(HttpServer *server, HttpMiddleware *middleware); where HttpMiddleware has a pointer to a callback that HttpServer would call at the beginning of serveRequest function. Here is a propotype:

typedef bool (*MiddlewareCallback)(HttpServer *server, HttpResponse *response, HttpMiddleware *middleware);

typedef struct HttpMiddleware {
  HttpMiddlewareCallback middlewareFunction;
  ///...
  HttpMIddleware *next;
} HttpMiddleware;

struct HttpServer {
  //...
  HttpMiddleware *middlewareList; // list of middleware
}
// in zss
HttpMiddleware *middleware = makeMiddleware(...);
middleware->middlewareFunction = myFunction;
registerHttpMiddleware(server, middleware);

Edit (@DivergentEuropeans ) : This has now been addressed

@DivergentEuropeans DivergentEuropeans changed the base branch from master to staging June 5, 2021 01:37
@DivergentEuropeans DivergentEuropeans changed the title (WIP) Some prototype RBAC checking + unfinished comments Introduce RBAC checking via custom authHandler function Jun 7, 2021
@DivergentEuropeans DivergentEuropeans marked this pull request as ready for review June 7, 2021 14:04
Signed-off-by: Leanid Astrakou <[email protected]>
Signed-off-by: Leanid Astrakou <[email protected]>
Signed-off-by: Leanid Astrakou <[email protected]>
Leonty Chudinov added 6 commits June 23, 2021 17:49
Signed-off-by: Leonty Chudinov <[email protected]>
Signed-off-by: Leonty Chudinov <[email protected]>
Signed-off-by: Leonty Chudinov <[email protected]>
Signed-off-by: Leonty Chudinov <[email protected]>
c/httpserver.c Outdated Show resolved Hide resolved
c/httpserver.c Show resolved Hide resolved
c/httpserver.c Outdated Show resolved Hide resolved
Leonty Chudinov and others added 2 commits August 31, 2021 12:54
…gisterHttpAuthorizationHandler

Add return code for registerHttpAuthorizationHandler
h/httpserver.h Outdated Show resolved Hide resolved
Leonty Chudinov and others added 2 commits August 31, 2021 13:48
…andler

Rename AuthorizationHandler to HttpAuthorize
Copy link
Contributor

@ifakhrutdinov ifakhrutdinov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, @lchudinov please also have a look, I think you're more familiar with the changes and the goals of those changes

Copy link
Contributor

@ifakhrutdinov ifakhrutdinov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, @rocketjared, perhaps you're interested in these changes.

@lchudinov, please re-review, it's always good to go over changes related to security multiple times

if (!head) {
server->authorizationHandlerList = handler;
} else {
while (head->next != NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A nit: you could've used first/last pointers to do O(1) insertion:

if (server->firstAuthHandler) {
  server->lastAuthHandler->next = handler;
} else {
  server->firstAuthHandler = handler;
}
server->lastAuthHandler = handler;

Or if you don't care about the current order:

handler->next = server->authorizationHandlerList;
server->authorizationHandlerList = handler;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation not so critical for now as we have only one authorization handler. It can be improved in the future when we for sure know what we want to optimize.

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