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

clasp login --creds emits poor error messages #526

Open
tehhowch opened this issue Feb 8, 2019 · 2 comments
Open

clasp login --creds emits poor error messages #526

tehhowch opened this issue Feb 8, 2019 · 2 comments

Comments

@tehhowch
Copy link

tehhowch commented Feb 8, 2019

Expected Behavior

Passing an incorrect file (any file that (currently) is not an installed app credentials file) to the --creds option results in clasp rejecting the file with a meaningful error, such as

Unable to obtain client ID & client secret from file $file

Actual Behavior

Generic error messages are returned, such as

Error retrieving access token: TypeError: Cannot read property 'project_id' of undefined

No .clasp.json settings found. create or clone a project first.

Steps to Reproduce the Problem

  1. The "no clasp.json" error occurs even for bad input files, but should probably first fail due to an improper argument.
~ $ clasp logout
~ $ clasp login
Logging in globally...
🔑 Authorize clasp by visiting this url:
https://accounts.google.com/o/oauth2/v2/auth?access_type=offline&scope=https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fscript.deployments%20https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fscript.projects%20https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fscript.webapp.deploy%20https%3A%2F%2Fwww.googleapis.com%2Fauth%2 Fdrive.metadata.readonly%20https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fdrive.file%20https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fservice.management%20https%3A%2F%2Fwww.googleapis.com%2Fauth%2Flogging.read% 20https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fcloud-platform&response_type=code&client_id=stuff.apps.googleusercontent.com&redirect_uri=http%3A%2F%2Flocalhost%3A61590

Authorization successful.

Default credentials saved to: ~\.clasprc.json (C:\Users\<user>\.clasprc.json).

~ $ clasp login --creds totally-not-credentials.txt
Warning: You seem to already be logged in *locally*. You have a ./.clasprc.json
Logging in locally...

No .clasp.json settings found. `create` or `clone` a project first.                                                                                                                                       
  1. If there is a .clasp.json file present, but no appsscript.json, the manifest error occurs after the message about logging in locally. As above, this should probably fail first for the bad argument:
~/<project> $ clasp login --creds totally-not-credentials.txt
Logging in locally...
Manifest: appsscript.json invalid. `create` or `clone` a project first.
  1. If there is also an appsscript.json manifest in the directory, then the situation referred to in this Stack Overflow post occurs:
~/<project> $ clasp status
Not ignored files:
└─ appsscript.json
   ...

Ignored files:
└─ .clasp.json

~/<project> $ clasp login --creds .clasp.json
Logging in locally...
Authorizing with the following scopes:
https://www.googleapis.com/auth/script.webapp.deploy

NOTE: The full list of scopes you're project may need can be found at script.google.com under:
File > Project Properties > Scopes

Error retrieving access token: TypeError: Cannot read property 'project_id' of undefined

The input file is assumed to be valid credentials by authorize:

clasp/src/auth.ts

Lines 88 to 98 in dde4fb2

if (options.creds) {
// if we passed our own creds
// Use local credentials
console.log(LOG.CREDS_FROM_PROJECT(options.creds.installed.project_id));
const localOAuth2ClientOptions: OAuth2ClientOptions = {
clientId: options.creds.installed.client_id,
clientSecret: options.creds.installed.client_secret,
redirectUri: options.creds.installed.redirect_uris[0],
};
oAuth2ClientOptions = localOAuth2ClientOptions;
} else {

and thus the log step triggers the error and catch branch:

clasp/src/auth.ts

Lines 180 to 182 in dde4fb2

} catch (err) {
logError(null, ERROR.ACCESS_TOKEN + err);
}

Specifications

  • Node version (node -v): v8.11.2
  • Version (clasp -v): 2.0.1
  • OS (Mac/Linux/Windows): Win10

If this isn't worth a code fix, I recommend making the expectations regarding the local credential file more obvious in the associated README - right now there is very little description:

Login
Logs the user in. Saves the client credentials to a .clasprc.json file.

Options
--no-localhost: Do not run a local server, manually enter code instead.
--creds : Use custom credentials used for clasp run. Saves a .clasprc.json file to current working directory. This file should be private!

Even under the main README's description of clasp run, there is no link to the much more helpful docs/run.md which does indicate where this local credential file should come from. (There is a link to it in the README's ToC, but not in the description of clasp run)

@campionfellin
Copy link
Collaborator

Hey @tehhowch thanks for the detailed report! I think the reason you are seeing different behavior in the 3 use cases you described is because they go through the flow of things in somewhat different orders. See:

  1. It sees your current credentials (this is fine). Then tries to read manifest, here:

const manifest = await readManifest();

This tried to read the local .clasp.json file,

clasp/src/manifest.ts

Lines 20 to 21 in b6a57db

export async function readManifest(): Promise<Manifest> {
let { rootDir } = await getProjectSettings();

  1. As you pointed out, with a .clasp.json file, it fails a few lines later here:

clasp/src/manifest.ts

Lines 26 to 28 in b6a57db

} catch (err) {
logError(null, ERROR.NO_MANIFEST(manifest));
throw Error('Could not read the manifest file.'); // TODO standardize errors.

  1. Finally, this looks like it is happening due to a difference in formatting between the auth file you get from the Gcloud console and from clasp login.

I think (1) and (2) can be grouped together and can be fixed by changing the order in which we make checks (like validating the file first).

(3) should be better documented and a new validation should be implemented for these types of auth files.

@swvajanyatek
Copy link

This seems to still be an issue?

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

No branches or pull requests

3 participants