transloadit / uppy

The next open source file uploader for web browsers :dog:
https://uppy.io
MIT License
29.14k stars 2.01k forks source link

Issue Integrating Companion Middleware Next.js #4727

Open cssetian opened 1 year ago

cssetian commented 1 year ago

Initial checklist

Link to runnable example

No response

Steps to reproduce

I am trying to run the uppy companion server as an express middleware in a nextjs server, where I receivce a 404 error upon redirect after completing the OAuth.

Some notes on my setup:

Here are 2 relevant gists for code examples:

Here some of the key companion server env vars and my current oath config for Google Drive:

Companion Env Vars:
export COMPANION_SECRET="secret-key"
export COMPANION_DOMAIN="localhost:3020"
export COMPANION_DATADIR="mnt/uppy-server-data"
export COMPANION_OAUTH_DOMAIN="http://localhost:3020"

Companion Google Drive Env Vars
// Copied from 'Client ID' of GCP Credentials Configuration
export GOOGLE_DRIVE_KEY="<secret_redacted>.apps.googleusercontent.com"
// Copied from 'Client Secret' of GCP Credentials Configuration
export GOOGLE_DRIVE_SECRET="<secret_redacted>"

Google Cloud API Credentials Configuration
Client ID Config (Configured as 'Web Application'):
Authorized Javascript origins:
http://localhost:3020
http://localhost:3000

Authorized Redirect URLs:
http://localhost:3020/companion/drive/redirect
http://localhost:3020/companion/dropbox/redirect

Oath Scopes Added
Google Drive API | .../auth/drive.install
Google Drive API | .../auth/drive
Google Drive API | .../auth/drive.metadata

Reproduction Steps:

Here is a screenshot of my network requests upon selecting the Google Drive integration, which seemed expected given the app has not been authenticated yet but posting in case its not expected:

Screenshot 2023-10-05 at 2 08 08 PM

After searching through numerous issues, posts, and forums, and reading through the oath docs, I cannot find anything that details what might cause this issue upon redirect. I thought that this may be an issue due to the uppy companion server being on a different port from the nextjs app, but it seems that it may be an issue on the companion server side with not knowing how to complete the redirect.

Any insights or help in debugging this would be appreciated, and can propose updates / clarifications to any relevant examples if there's a solution where it would help. I am happy to push a more fully working example with the 2 servers, but for the sake of initial investigation only posted the relevant code samples as I will need to redact other parts of the app in order to post a minimal but fully functioning set of runnable servers.

Expected behavior

Actual behavior

Murderlon commented 1 year ago

Hi, this seems to be an issue specifically with integrating it into Next.js. Regarding your client-side code, you are integrating Uppy incorrectly. Uppy can not be directly declared inside a component. Either move it outside the component or put it in useState with an initialiser function. This is likely to cause problems already.

Regarding the server-side, perhaps @mifi seems something going wrong there.

mifi commented 1 year ago

Hi. Did you try first to run it as stand-alone mode? If you can get stand-alone working, then it might be easier to work from a working standalone setup and move over to middleware, because the middleware mode is more complicated to get everything right. If you are able to get stand-alone working, then could you try first a normal express app (without next.js) and see if you can get it working, then it might be easier to take it from there and move it to next.js? we haven't ever tested with next.js so i'm not sure how that would even work.

cssetian commented 1 year ago

@Murderlon Thanks for the responses. Regarding the client side code, I take it is that because we only want to instantiate it once? Could I memoize it in the following way and take the same effect that you suggest with useState?

export function FileUploader({
  onSuccess,
  userId,
  companionUrl
}: {
  onSuccess: (files: Document[]) => void
  userId: string
  companionUrl: string
}) {
  const { theme } = useTheme()
  const uppy = useMemo(
    () =>
      new Uppy({
        id: 'uppy1',
        debug: true,
        autoProceed: false,
        restrictions: {
          maxFileSize: 10000000,
          maxNumberOfFiles: 30,
          minNumberOfFiles: 1,
          allowedFileTypes: null
        }
      })
        .use(GoogleDrive, { companionUrl })
        .use(Dropbox, { companionUrl })
        .use(OneDrive, { companionUrl })
        ........,
    [userId, companionUrl, onSuccess]
  )

  return (
    <Dashboard
      uppy={uppy}
      plugins={['GoogleDrive', 'Dropbox', 'OneDrive']}
    />
  )
}

@mifi Thanks for the input around the server. Regarding the server side code, I didn't try the stand alone because I want to add a certain degree of customization and security on the server, and I want to be able to add some server side authentication middleware to check that external requests being made to the sever are coming from authenticated clients (within my nextJS app). I chose to try to host it on NextJS for ease of infrastructure management.

I still have not managed to get this working, but managed to successfully deploy the application as middleware to a Google App Engine based vanilla express app. Here is an updated gist that shows the configuration of the app on GAE. I use google secret manager to store env vars that need to be maintained as secrets and injected into the app, along with application default credentials to inject them. The app.yaml file illustrates non-secret env vars which are injected upon deployment.

I would be interested to know if my configuration of uploadUrls, host, oauthDomain, validHosts, and (lack of) path are expected/valid though for a self hosted companion. It was a little difficult to figure out what exactly needed to be done from the docs and I effectively had to do trial and error to get it working correctly. Thanks for the help!

Murderlon commented 1 year ago

Could I memoize it in the following way and take the same effect that you suggest with useState?

You may get away with it if you dependencies never change when users interact with Uppy. Otherwise your uploads will be abruptly cancelled during a re-render. To be on the safe side you can do this:

// companionUrl can be an environment variable? Doesn't need to live inside React?
// onSuccess is constant and never changes?
// important: passing an initialiser function
const [uppy] = useState(() => new Uppy().on('upload-success', onSuccess))

// note that if userId is available during the initial render and won't change afterwards,
// you don't need this effect
useEffect(() => {
  if (userId) {
    // Adding to global `meta` will add it to every file (not sure what you want)
    uppy.setOptions({ meta: { userId } })
  }
}, [userId])

If onSuccess does change based on some state you can do this instead

useEffect(() => {
  uppy.on('upload-success', onSuccess)
  return () => uppy.off('upload-success', onSuccess)
}, [uppy, onSuccess])