-
Notifications
You must be signed in to change notification settings - Fork 984
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
WIP: setup auth <npm-package> #7430
Conversation
16 replays were recorded for 24c0729. 16 Passed
requireAuth graphql checks
|
8c78b60
to
1d1ddf8
Compare
Hey Tobbe, just a reminder we need to be careful about allowing any packages to be installed! IMO, plugins to setup anything (auth or otherwise) needs to be an active step, such as enabling a list of plugins in the toml, or via CLI. e.g.
Which would prompt and confirm the addition to a redwood.toml, and install a package to the root package.json. You also have the option of checking version compatibility at this point. +[plugins]
+ - redwood-auth-bazinga This creates a path to extract not only auth, but other setup commands too - and potentially opens up "hooks" in our cli. It would reduce the chance that a user installs a mailicious package, but also avoids the need to pull things off the internet during the setup process (since the package is locally installed at this point) I described the concept here: #2086 (comment) |
@dac09 When we do plugins we probably need to discuss the dx more with the team. For now I'm trying to keep the scope small and just do something for auth. |
Danny and I talked. We decided we need to do something to protect users from just installing some random package not realizing it might be a good idea. Better be too strict now, and loosen up later, than the other way around after something bad has already happened. One idea was to prompt users when setting up auth using a package that's not from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still reviewing, but will post one comment while I have it: tried yarn rw setup auth @redwoodjs/auth-gotrue
and it doesn't seem to work quite yet, I think because @redwoodjs/auth-gotrue
isn't a package—it's just a prefix (the real packages are @redwoodjs/auth-gotrue-{api,web,setup}
.
output
dom@evaM1 ~/p/r/redwood-app (prs/setup-auth) [1]> yarn rw setup auth @redwoodjs/auth-gotrue
Set up auth using @redwoodjs/auth-gotrue
rw setup auth
Commands:
rw setup auth build [side..] Build for production
rw setup auth check Get structural diagnostics for a Redwood
project (experimental)
[aliases: diagnostics]
rw setup auth console Launch an interactive Redwood shell
(experimental) [aliases: c]
rw setup auth data-migrate <command> Migrate the data in your database
[aliases: dm, dataMigrate]
rw setup auth deploy <target> Deploy your Redwood project
rw setup auth destroy <type> Rollback changes made by the generate
command [aliases: d]
rw setup auth dev [side..] Start development servers for api, and
web
rw setup auth exec [name] Run scripts generated with yarn generate
script
rw setup auth generate <type> Generate boilerplate code and type
definitions [aliases: g]
rw setup auth info Print your system environment
information
rw setup auth lint [path..] Lint your files
rw setup auth prerender Prerender pages of your Redwood app at
build time [aliases: render]
rw setup auth prisma [commands..] Run Prisma CLI with experimental
features
rw setup auth record <command> Setup RedwoodRecord for your project.
Caches a JSON version of your data model
and adds api/src/models/index.js with
some config.
rw setup auth serve [side] Run server for api or web in production
rw setup auth setup <command> Initialize project config and install
packages
rw setup auth storybook Launch Storybook: a tool for building UI
components and pages in isolation
[aliases: sb]
rw setup auth test [filter..] Run Jest tests. Defaults to watch mode
rw setup auth ts-to-js Convert a TypeScript project to
JavaScript
rw setup auth type-check [sides..] Run a TypeScript compiler check on your
project [aliases: tsc, tc]
rw setup auth upgrade Upgrade all @redwoodjs packages via
interactive CLI
Options:
--help Show help [boolean]
--version Show version number [boolean]
--cwd Working directory to use (where `redwood.toml` is located)
Examples:
yarn rw g page home / "Create a page component named 'Home' at path '/'"
Error: Command failed with exit code 1: yarn npm info @redwoodjs/auth-gotrue --fields versions --json
{"type":"error","name":35,"displayName":"YN0035","indent":"","data":"The remote server failed to provide the requested resource"}
{"type":"error","name":35,"displayName":"YN0035","indent":"","data":" Response Code: 404 (Not Found)"}
{"type":"error","name":35,"displayName":"YN0035","indent":"","data":" Request Method: GET"}
{"type":"error","name":35,"displayName":"YN0035","indent":"","data":" Request URL: https://registry.yarnpkg.com/@redwoodjs%2fauth-gotrue"}
at makeError (/Users/dom/prjcts/redwood/redwood-app/node_modules/execa/lib/error.js:60:11)
at Function.module.exports.sync (/Users/dom/prjcts/redwood/redwood-app/node_modules/execa/index.js:194:17)
at Function.module.exports.commandSync (/Users/dom/prjcts/redwood/redwood-app/node_modules/execa/index.js:235:15)
at getLatestAuthSetup (/Users/dom/prjcts/redwood/redwood-app/node_modules/@redwoodjs/cli/dist/commands/setup/auth/auth.js:218:22)
at getAuthSetup (/Users/dom/prjcts/redwood/redwood-app/node_modules/@redwoodjs/cli/dist/commands/setup/auth/auth.js:206:10)
at Object.handler (/Users/dom/prjcts/redwood/redwood-app/node_modules/@redwoodjs/cli/dist/commands/setup/auth/auth.js:137:15)
at /Users/dom/prjcts/redwood/redwood-app/node_modules/yargs/build/index.cjs:1:8891
at /Users/dom/prjcts/redwood/redwood-app/node_modules/yargs/build/index.cjs:1:4949 {
shortMessage: 'Command failed with exit code 1: yarn npm info @redwoodjs/auth-gotrue --fields versions --json',
command: 'yarn npm info @redwoodjs/auth-gotrue --fields versions --json',
escapedCommand: 'yarn npm info "@redwoodjs/auth-gotrue" --fields versions --json',
exitCode: 1,
signal: undefined,
signalDescription: undefined,
stdout: '{"type":"error","name":35,"displayName":"YN0035","indent":"","data":"The remote server failed to provide the requested resource"}\n' +
'{"type":"error","name":35,"displayName":"YN0035","indent":"","data":" Response Code: 404 (Not Found)"}\n' +
'{"type":"error","name":35,"displayName":"YN0035","indent":"","data":" Request Method: GET"}\n' +
'{"type":"error","name":35,"displayName":"YN0035","indent":"","data":" Request URL: https://registry.yarnpkg.com/@redwoodjs%2fauth-gotrue"}',
stderr: '',
failed: true,
timedOut: false,
isCanceled: false,
killed: false
}
|
||
async function getLatestAuthSetup(module, ignoreScripts) { | ||
const { stdout } = execa.commandSync( | ||
`yarn npm info ${module} --fields versions --json` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is where the command failed when I tried it, because @redwoodjs/auth-gotrue
isn't a package
() => {}, | ||
async () => { | ||
// Here be dragons... | ||
// There's no way for the user to actually end up here, but we need | ||
// this command to get the help output we want from yargs | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The help still seems to show when I remove the builder and handler here (() => {}
and async () => {}
); doesn't seem to matter if we leave them in or not, but the .command()
is definitely necessary. But maybe we could move the comment up to before .command()
to reflect that?
// This is a workaround for https://github.com/yargs/yargs/issues/2291 | ||
if (args._.length <= 2) { | ||
let helpCmd = 'yarn rw setup auth --help' | ||
|
||
if (/cli[\/\\]dist[\/\\]index.js/.test(process.argv[1])) { | ||
helpCmd = 'yarn dev setup auth --help' | ||
} | ||
|
||
console.error(execa.commandSync(helpCmd).stdout) | ||
process.exit(1) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need this block now that that issue you linked to is closed?
() => {}, | ||
async (args) => { | ||
// This is a workaround for https://github.com/yargs/yargs/issues/2291 | ||
if (args._.length <= 2) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it ever just not be 2? This handles the case where the user does yarn rw setup auth
right? Or maybe you're handling yarn dev
here?
Superseded by #8920 |
This PR adds the ability to set up auth using any setup package that's published to npm. It supports both
npm-package
andnpm-package@version
to install the latest version or the specific version given.Open issue on yargs about a problem I'm running in to:
yargs/yargs#2291