refactor: update nf1 example with new notehub js oauth workflow [DO NOT MERGE]

[paigen11]

Problem Context

The Notehub JS library has been updated to use OAuth bearer tokens for its authentication with the Notehub API, and so our sample code using the library to interact with the Notehub API must be updated accordingly.

Changes

• Replace HUB_AUTH_TOKEN env var with HUB_CLIENT_ID and HUB_CLIENT_SECRET - these are the two bits of data used to generate an expiring Oauth bearer token via Notehub
• Update the version of the Notehub JS npm library the project is using to take advantage of the new generateAuthToken() function and bearer token auth method for all subsequent requests.
• Create a new generateNotehubAuthToken() function inside of the NotehubDataProvider.ts file that will be called before any other calls to the Notehub API to generate a fresh token for the project and fetch data. Use it in this file and in the NotehubAttributeStore.ts file to make updates to the project.
• Remove all now unused references to the HUB_AUTH_TOKEN in the code
• Update readme.md file with details on how to get the client_id and client_secret from a Notehub project for use in the app.

Screenshot (if applicable)

App is using Notehub JS library to fetch data from Notehub.

Testing

Unit / integration / e2e tests?

All tests pass and code compiles.

Steps to test manually?

1. Grab a HUB_CLIENT_ID and HUB_CLIENT_SECRET from an Notehub project and add them to the .env and .env.local project files.
2. Fire up the app and go to http://localhost:4000
3. Route an event or two to the app
4. See the data show up in the UI (the Notehub JS library's taking care of all that)

Any other sorts of testing notes to include?

Any other related PRs

n/a

Ticket(s)

n/a

[CarltonHenderson]

Some questions about token renewal.

[CarltonHenderson]

any? really?

[paigen11]

What type would you recommend for a client library like this with a bunch of functions it provides?

[CarltonHenderson]

A few options, depending on how you see it.

1. Add types to notehub-js so we can import the appropriate type. (Eventually)
2. Make a new type in NotehubAttributeStore that represents just parts of Notehub-js that NotehubAttributeStore depends on. Then you've documented what you're expecting to be provided by the notehub-js and when we finally add types to notehub-js we'll have the benefit of typescript checking to make sure we are depending on the same type of thing as notehub-js is actually providing.
3. Keep using any as you're doing now. This does make a fair amount of sense now that I think about it. In a way it's a good red flag that we're using a library without typescript support.

[paigen11]

I'd prefer to leave it as any for now and circle back to it when I've had a chance to add a .d.ts file to the Notehub JS library

[CarltonHenderson]

You're generating a new auth token on every call? I thought the idea was to generate the auth token once every 30 minutes and just renew it right before it's about to expire (or even after it expires, right before you're about to need it). Actually, feels like something the notehub-js library should do for you.

[paigen11]

For the sake of getting this out quickly as a sample for interested users to peruse, but since the OAuth approach is being reviewed again, I've asked the team if they can include some property on the token that will make it easier for me to determine when a token refresh is actually needed before a Notehub API call.

[tjvantoll]

The call that generates the token also returns an expires_in, which this app could store and use to conditionally generate tokens.

Or the library could do that. Still debating in my head whether having the library do this magically would be good or not.

[tjvantoll]

@paigen11 Ah sorry typed my comment before seeing yours. We do have this line in the docs.

Tokens automatically expire after 30 minutes, as reflected by the expires_in property (in seconds), which is returned alongside the access_token.

Is expires_in not actually there?

[paigen11]

It is. I'll take a closer look at it next week and clean this up some more.

[paigen11]

Refactored to store auth token data (including a custom generated token_expiration_date property) inside of cookies. Each time an API call is made anywhere in the app, the cookie's presence and validity are checked and if it's not set or expired a new auth token is generated, the API call is made, and the new auth data overwrites the previous cookie data (if there was one)

[paigen11]

@CarltonHenderson and @tjvantoll feel free to take another look at this PR at your convenience (it won't be merged until after the OAuth workflow for the Notehub API has been revised).

Now, I've refactored the app to store Oauth token data (including a custom generated token_expiration_date property) inside of cookies. Each time an API call is made anywhere in the app, the cookie's presence and validity are checked and if the cookie is not set or its auth data is expired according to the token_expiration_date property, a new auth token is generated, the API call is made, and the new auth data overwrites the previous cookie's auth data (if there was one)

[CarltonHenderson]

Looks pretty good. Nice idea to use cookies since the web host is 'serverless'. See some comments on code structure and other possible improvements. I still wonder if some of this could be or should be done by notehub-js. (not the next-js or cookie-specific parts, but the token checking, renewing, and abstract storing, etc. -- that is, everything that every customer of notehub-js will be burdened to do if we leave the example as it sits here)

[CarltonHenderson]

Please, break this out into a few functions in order to:

• DRY up the code that's currently duplicated ~5 times.
• Separate out the next-js specific code.
• Separate out the cookie-specific code (in case someone who's not server-less prefers to use a filesystem, global variable, redis etc.)
• Separate out any other separate concerns you notice along the way. Each function should do one thing and do it well.

Those changes should make this code a lot easier to read, understand, review, test, etc.

Bonus points (overkill for our toy app, but very important in some customer apps):

• Consider adding encryption so the notehub auth token is never in plaintext in the client's browser.

[paigen11]

Code dried up:

• Added auth-token.ts file for reusable token generation / verification functions on client-side API requests. Now, all client API calls (update env vars, update device name, etc.), call this function first to make sure their auth tokens are in order.
• Added cookieAuth.ts file for reusable functions to handle fetching, storing and transforming auth tokens as cookies.
• Used reusable functions from first two files mentioned to dry up code for index.tsx and settings.tsx pages getServerSideProps functions.

[CarltonHenderson]

Remind me the difference between AppModel and DomainModel?

Do you need to store expires_in and token_expiration_date? Seems like expires_in will start to go stale as soon as you store it whereas token_expiration_date will be accurate forever.

[paigen11]

AppModel is client side, app-specific objects, domain model is server side, more generic objects that you could fetch from a database, Notehub API, etc.

This object goes from server side to client side and back again, so I added it to AppModel but the case could be made for either file.

expires_in is one of the properties the token comes generated at creation, token_expiration_date is a property I manually calculate based on the current time and expires_in. And you're correct, it goes stale after initial generation.

I'm planning to ask the Notehub team if they could include something like a timestamp akin to token_expiration_date in addition to expires_in so we don't have to do the math for it.

[CarltonHenderson]

I like the 'functional' aspect of adding an authToken argument to each function, but might it save some typing/reading/thinking to just have a setAuthToken: (AuthToken) => {}; function (maybe it replaces the checkAuthTokenValidity too and just throws an error if the token is invalid?) on the AppServiceInterface? I'm really on the fence here.

Remember to think from the highest-possible-level of abstraction for what makes sense there. Don't change the app layer or domain layer too precisely to suit the needs of the lower-level implementation details. If you must add something to the domain or app layer, do it in a very generic way and let lower levels implement and inject details as they see fit.

I'm trying to explain a lot in a small number of words here. If this is enough detail, great, but if you want to huddle on slack let me know. Thanks for your patience and persistence here so we can make this example project exemplary.

[paigen11]

@CarltonHenderson just to make sure I'm understanding what you're saying: are you recommending something along the lines of this inside of AppService.ts?

interface AppServiceInterface {
  setAuthToken: (AuthToken) => {};
  // other functions...
  getDeviceTrackerData: () => Promise<DeviceTracker[]>;
}

 setAuthToken: (AuthToken) {
    return this.dataProvider.setAuthToken();
  }

  async getDeviceTrackerData() {
    const authToken = this.dataProvider.setAuthToken();
    return this.dataProvider.getDeviceTrackerData(authToken);
  }

[CarltonHenderson]

Almost... more like

interface AppServiceInterface {
  setAuthToken: (AuthToken) => {};
  // other functions...
  getDeviceTrackerData: () => Promise<DeviceTracker[]>;
}

 setAuthToken: (AuthToken) {
    this.authToken = AuthToken;
  }

  async getDeviceTrackerData() {
    const authToken = this.dataProvider.setAuthToken(this.authToken);
    return this.dataProvider.getDeviceTrackerData(authToken);
  }

[CarltonHenderson]

Or like this

interface AppServiceInterface {
  setAuthToken: (AuthToken) => {};
  // other functions...
  getDeviceTrackerData: () => Promise<DeviceTracker[]>;
}

 setAuthToken: (AuthToken) {
    this.authToken = this.dataProvider.setAuthToken(AuthToken);
  }

  async getDeviceTrackerData() {
    return this.dataProvider.getDeviceTrackerData(AuthToken);
  }

Not sure which makes more sense in this circumstance.

Actually, what probably makes most sense would be making a dataProvider that does its own auth (whether or not the notehub-js handles some of the auth + token renewal) so the AppServiceInterface doesn't need to change at all. Let me know if you want me to really put pen to paper and help figure out which one of these makes most sense.

[CarltonHenderson]

I hope you appreciate being given four options and freedom to choose your own adventure. Let me know if you want to pair on it. It's not that easy to figure out what will give the easiest understanding and future flexibility.

[paigen11]

My first attempts at making the NotehubDataProvider do its own auth is what ended up being a non-starter due to the fact that the OAuth token values weren't being preserved server-side between app refreshes.

I too would like to make the AppServiceInterface unchanged, but I'm just not sure it's possible. Today, I addressed your comments above and DRYed up the client side code, tomorrow, I'll try to look more deeply into these suggestions and may ping you to discuss.

[paigen11]

Net net after pairing with Carl, we're going back to basics: new OAuth token every API call for this app to avoid all the extra hoops this app is making us jump through to store tokens until they expire.

I'll update another app that uses a Prisma DB to store tokens there and demonstrate one way to store that data in a table and periodically update it as needed all on the server side. The client shouldn't need to know about it all

[paigen11]

Closing this PR as the Notehub auth system now creates long-lived project tokens and handles token refreshing on its end when needed so we don't have to handle it on our end.