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.

Sign up for GitHub

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

Jump to bottom

fix(domains/overview): Avoid throwing an error if the user is not allowed to view application domains (accountants for instance) #882

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

fix(domains/overview): Avoid throwing an error if the user is not allowed to view application domains (accountants for instance) #882

wants to merge 1 commit into from

Conversation

florian-sanders-cc
Copy link
Contributor

@florian-sanders-cc florian-sanders-cc commented Feb 24, 2025 •
edited
Loading

Fixes #881

What does this PR do?

  • Catches 403 errors when trying to list domains of an app.
  • Returns an empty array in such cases (apps with no domains are filtered out from the command output).

Note that the Clever access management is weird: accountants may list applications but they cannot access info about them.
This is a quick and easy solution.

We could try and do better by printing messages for apps that users cannot access but this would require heavier refactoring and it's not a very interesting thing to do with the current Clever Cloud roles.
Only people with the Accountant role are really impacted by this and they are not really the audience of the CLI and this specific command.

How to review?

  • Set yourself as an "accountant" in one of your orgs,
  • Try to run the clever domain overview with the prod CLI
    • You get an error,
  • Try to run the same command with this branch / preview,
    • You see all apps you can access and no error 🎉

@florian-sanders-cc
fix(domains/overview): silence 403 errors
0dd36ca 
Avoid throwing an error and exiting the process if the user is not
allowed to view application domains (accountants for instance).
Only display domains for apps that the user is allowed to view

Fixes #881
@florian-sanders-cc florian-sanders-cc requested a review from a team as a code owner February 24, 2025 12:40
@github-actions GitHub Actions
Copy link

🔎 A preview has been automatically published:

  • 🐧 linux 57c384793defc9eebe71d112ab01794d4f281ef7980b71247615b9efea19a9a4
  • 🍏 macos 923b08799a70a9acb43c374383d6de9619ed84657309b2d27ac433a86881ecb3
  • 🪟 win faf388ab9735846d974b0d95bcb57655f6fd57664916e05b4e5245b430377ec1

This preview will be deleted once this PR is closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@hsablonniere hsablonniere

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

domain overview: command fails if you lack privileges in at least one of your organization
2 participants
@florian-sanders-cc @hsablonniere