psolymos / clickrup

Interacting with the ClickUp v2 API from R
https://peter.solymos.org/clickrup/
MIT License
18 stars 3 forks source link

Allow direct API keys? #19

Closed jonthegeek closed 7 months ago

jonthegeek commented 7 months ago

TIL: Environment variables set via Sys.setenv() are shared across all Shiny sessions (as are options). This isn't SUPER surprising (they share a global environment), but it means we can't use clickrup for user-specific things in a Shiny app; everyone using the app would share/update the CU_PAT environment variable.

Would it be feasible to add something like a cu_api_key argument to every function, which defaults to Sys.getenv("CU_PAT")? In Shiny calls, we could set that argument explicitly.

This may be moot if I'm able to regenerate the package from the API definition.

psolymos commented 7 months ago

We can probably just make it implicit and pass it through ... to the internals that deal with the PAT stuff.

jonthegeek commented 7 months ago

Something along those lines could make sense. It PROBABLY wouldn't require a huge rewrite, but I didn't dig in all the way to make sure!

psolymos commented 7 months ago

@jonthegeek I checked the internals and passing the PAT would mean adding an argument to each api function and pass it to the internals.

Question: The reason one uses an env variable is because we do not want to expose the token. Would users add their token as part of the Shiny UI?

I wonder if the OAuth flow would be more appropriate or this -- where users would be redirected to ClickUp to log in, and directed back to the app with the access token as a query parameter that Shiny can grab. The only issue is that the redirect URL cannot be specific to a shiny session, thus session affinity could be an issue if before-auth state is important.

Thoughts?

jonthegeek commented 7 months ago

I've implemented the oauth flow. In the end, the user receives a token. We need to pass that token to clickrup. The rest of the flow works, we just can't use the environment variable, because then each time someone logs in everyone using the app switches to their token for the next call.

psolymos commented 7 months ago

I see. Very inconvenient.

jonthegeek commented 7 months ago

It has been a good learning experience! I never thought about the fact that all shiny sessions are environments under the same global, so Sys.setenv() and options() are shared across all users. It opens up intriguing possibilities for another mechanism for sharing information amongst users, but it's annoying for this case!

psolymos commented 7 months ago

You can check the cu-token branch.

jonthegeek commented 7 months ago

LOL I had review comments, but I finally fell back asleep and then dreamt that I sent them 🙃

It's definitely an improvement! I'll test it out as soon as I get a chance!