Skip to content
This repository was archived by the owner on Jun 12, 2019. It is now read-only.

check if defaults.json exists on auth and check for success status codes#45

Merged
jeremytregunna merged 4 commits intomasterfrom
ct-defaults-check-auth
Oct 3, 2018
Merged

check if defaults.json exists on auth and check for success status codes#45
jeremytregunna merged 4 commits intomasterfrom
ct-defaults-check-auth

Conversation

@eecaro
Copy link
Copy Markdown
Contributor

@eecaro eecaro commented Oct 3, 2018

⚠️ needs https://github.com/MetisMachine/argus/pull/318 ⚠️

  • When checking for API request errors, check that the status code does not exist in the list of success codes
  • When loading defaults, check that the org_name exists in the file (i.e. is not "")
  • When checking authentication, check that the defaults were able to be loaded then check token and access to org name
    • Not authenticated if
      • defaults.json file does not exist
      • defaults.json does exist but org_name is empty
      • user does not have access to organization listed in defaults.json file
    • api-token is invalid
    • credentials do not exist

Comment thread src/env/env.cpp
if (org_name.length() < 1){
console::error("Defaults error: no default organization found.");
return false;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm wondering if we shouldn't just automatically create a defaults.json after we've polled the orgs endpoint instead, and select their personal organization as default if no other default exists. Thoughts? Could be wrapped into the auth flow even.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah that could be a smoother work flow for the user. Right now, If it didn't find a default, it would require you to authenticate then it would load the defaults. I kind of like the idea that when it checks for auth it would select your default or user organization if it wasn't. That being said, does that defeat the purpose of checking if they are authenticated/requiring them to reauth because we could potentially do it for the user's api-token as well?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hrmm... I'm honestly not sure. Let's ask Luke :)

@jeremytregunna jeremytregunna merged commit 9e6ac2c into master Oct 3, 2018
@jeremytregunna jeremytregunna deleted the ct-defaults-check-auth branch October 3, 2018 14:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants