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

feat: Extendable context #299

Merged
merged 7 commits into from
Dec 19, 2019
Merged

Conversation

willhausman
Copy link
Contributor

Proposal for #295 and #296 questions.

Ability to extend context object per request (http or web socket). Essentially, divide responsibilities of authInfo and context. This supports two approaches living side-by-side.

  1. Current approach, where every component of the request is considered an authenticated security context. "I certify that this user is id 4 and has a mole on his left cheek."
  2. Extended approach where authenticated request identifies a user, and additional request information can be added for customization a developer might need. "I certify that this user is id 4. Btw, he says he has a mole on his left cheek, if that matters to you."

requestId now lives within the context, can be accessed for requests made external to cubejs, and can be overridden if not using x-request-id header.

With API:

const server = new CubejsServer({
  checkAuth: (req, auth) => { req.authInfo = verify(auth); },
  extendContext: (req) => ({
    schemaName: req.query.schemaName, // TODO: verify if this is important
    hasMoleOnLeftCheek: req.headers['wh-hasMoleOnLeftCheek'] // validation irrelevant. informational
  }),
  contextToAppId({ authInfo }) => `${authInfo.tenantId}`,
  schemaVersion({ authInfo, schemaName }) => `${schemaName}-${authInfo.u.id}`,
  repositoryFactory({ authInfo, schemaName, hasMoleOnLeftCheek }) => {
    // implementation where we call a microservice using `authInfo.jwt` with `schemaName`
    // and `hasMoleOnLeftCheek` as additional parameters (route or query)
  }
});

Or with web sockets:

const server = new CubejsServer({
  checkAuth: (req, auth) => { req.authInfo = verify(auth); },
  extendContext: (message) => ({
    schemaName: message.params.schemaName, // TODO: verify if this is important
    hasMoleOnLeftCheek: message.params.hasMoleOnLeftCheek // validation irrelevant. informational
  }),
  contextToAppId({ authInfo }) => `${authInfo.tenantId}`,
  schemaVersion({ authInfo, schemaName }) => `${schemaName}-${authInfo.u.id}`,
  repositoryFactory({ authInfo, schemaName, hasMoleOnLeftCheek }) => {
    // implementation where we call a microservice using `authInfo.jwt` with `schemaName`
    // and `hasMoleOnLeftCheek` as additional parameters (route or query)
  }
});

The only difference is how you interact with extendContext(), and websockets will pass in { isSubscription: true }, so your server could handle both situations if it needed to.

const server = new CubejsServer({
  checkAuth: (req, auth) => { req.authInfo = verify(auth); },
  extendContext: (req) =>
    req.isSubscription ? { // web sockets
      schemaName: req.params.schemaName, // TODO: verify if this is important
      hasMoleOnLeftCheek: req.params.hasMoleOnLeftCheek // validation irrelevant. informational
    } : { // https
      schemaName: req.query.schemaName,
      hasMoleOnLeftCheek: req.headers['wh-hasMoleOnLeftCheek']
    },
  contextToAppId({ authInfo }) => `${authInfo.tenantId}`,
  schemaVersion({ authInfo, schemaName }) => `${schemaName}-${authInfo.u.id}`,
  repositoryFactory({ authInfo, schemaName, hasMoleOnLeftCheek }) => {
    // implementation where we call a microservice using `authInfo.jwt` with `schemaName`
    // and `hasMoleOnLeftCheek` as additional parameters (route or query)
  }
});

@willhausman
Copy link
Contributor Author

willhausman commented Dec 17, 2019

WIP - I wanted to get my thoughts out of my head and into code to solicit feedback before I forgot them. I have yet to actually test them.

@paveltiunov
Copy link
Member

@willhausman Hey Will! Looks good to me. The only thing to consider: as context becomes single point of fetching additional info for request processing it probably should be async then.

@willhausman
Copy link
Contributor Author

@paveltiunov Agreed.

@willhausman willhausman changed the title [WIP] feat: Extendable context feat: Extendable context Dec 19, 2019
@willhausman
Copy link
Contributor Author

@paveltiunov Alright, now it 's looking good from my end.

@paveltiunov
Copy link
Member

@willhausman Yeah. Great job! Thanks for contributing this!

@paveltiunov paveltiunov merged commit 38e33ce into cube-js:master Dec 19, 2019
@willhausman willhausman deleted the extendable-context branch December 20, 2019 21:44
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.

None yet

2 participants