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

Allow arena to work as a node module alongside auth middleware. #123

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aaron-blondeau-dose
Copy link

I have been unable to find a way to add arena to an existing express app while also implementing authentication. The basePath parameter conflicts with any path other than "/" provided to express' use method. Allowing users to provide a new parameter called "mountPath" can alleviate this issue and allow configs like the following to work:

const arena = Arena({
    queues
}, {
    basePath: '/admin/',
    mountPath: '/',
    disableListen: true
})

app.use('/admin', basicAuth('foo', 'bar'), arena)

where basicAuth = https://github.com/expressjs/basic-auth-connect

@javorosas
Copy link

I'm looking forward to this


app.use(app.locals.appBasePath, express.static(path.join(__dirname, 'public')));
app.use(app.locals.appBasePath, routes);
app.locals.mountPath = listenOpts.mountPath || app.locals.basePath;
Copy link

Choose a reason for hiding this comment

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

Unless I'm reading this incorrectly, you intended for this to be listenOpts.mountPath || app.locals.appBasePath;, right?

@dbronin
Copy link

dbronin commented Apr 30, 2019

Any updates on this?

@bogdan
Copy link
Contributor

bogdan commented Nov 18, 2019

I have no idea why it is not merged yet.
However, here is a workaround at the moment:

const arena = Arena({
    queues
}, {
    basePath: '/',
    disableListen: true
})

// powerhacking
arena.locals.appBasePath = "/admin"

app.use('/admin', basicAuth('foo', 'bar'), arena)

Copy link
Member

@skeggse skeggse left a comment

Choose a reason for hiding this comment

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

This has been a long-requested feature. I'd like to move this along, but still don't have a ton of time to review. I don't immediately understand the distinction between the appBasePath and the mountPath. Along those lines, could we get some documentation in the readme describing how the new option functions and how it differs from the old?

@skeggse
Copy link
Member

skeggse commented Aug 6, 2020

Merging #182 into this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants