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

Unable to upload documentation file #1646

Closed
philippeluickx opened this issue Sep 27, 2016 · 14 comments
Closed

Unable to upload documentation file #1646

philippeluickx opened this issue Sep 27, 2016 · 14 comments
Assignees
Labels
Milestone

Comments

@philippeluickx
Copy link
Contributor

philippeluickx commented Sep 27, 2016

On nightly.
Add a new API backend.
Go to documentation tab. Select manage. Select a sample json file to upload.
Unable to upload documentation for an API I own:

File creation failed! n {error: 403, reason: "Access denied", details: undefined, message: "Access denied [403]", errorType: "Meteor.Error"}
@philippeluickx
Copy link
Contributor Author

Also unable to upload logo

File creation failed! n {error: 403, reason: "Access denied", details: undefined, message: "Access denied [403]", errorType: "Meteor.Error"}

@bajiat bajiat added the bug label Sep 27, 2016
@bajiat
Copy link
Contributor

bajiat commented Sep 27, 2016

@marla-singer Can you take a look at this?

@marla-singer
Copy link
Contributor

@bajiat It's dublicate this issue #1570. I researched and detected that only users, who has a role 'admin' or 'manager', can upload a file. User doesn't have ane role on default. It can be solved in two ways:

  1. Administrator can share role 'manager' to user
  2. Or someone fixes it on code - if user add an api, will give him 'manager' role

@bajiat
Copy link
Contributor

bajiat commented Sep 27, 2016

@marla-singer It is true that user does not have any role by default. But when they add an API, they become a manager automatically - but only for that particular API. This should already work, unless there is regression.
@philippeluickx added an API, thus becoming a 'manager' for that API. The same thing in #1570, @saralavanip started by adding an API.

@marla-singer
Copy link
Contributor

@bajiat It looks like users don't become a manager automatically.

@bajiat
Copy link
Contributor

bajiat commented Sep 27, 2016

@brylie Can you check this - do we have a regression and user does not become a manager if they add an API? Or is it related to adding Swagger files only?

@brylie brylie self-assigned this Sep 27, 2016
@brylie brylie added the ready label Sep 27, 2016
@brylie brylie added this to the Sprint 32 milestone Sep 27, 2016
@philippeluickx
Copy link
Contributor Author

@bajiat It is also for uploading the logo, so I would assume that @marla-singer is correct in her assumption.

@brylie
Copy link
Contributor

brylie commented Sep 29, 2016

@bajiat, @marla-singer is correct that only users with Manager or Admin role can upload API Documentation files.

We can add a small fix for this issue, so that when they create an API they are assigned the 'manager' role.

// in apis/client/add/autoform.js
onSuccess (formType, apiId) {
      ...

      // Get current user ID
      const userId = Meteor.userId();

      // Give user manager role
      Roles.addUsersToRoles(userId, 'manager');
    }

We will also need an Allow rule for the Roles collection

// in /users/collection/roles_permissions.js
Meteor.roles.allow({
  insert () {
    // return true is insecure
    // we need to consider whether it is dangerous for users to generally invent roles.
    return true;
  },
});

@brylie
Copy link
Contributor

brylie commented Sep 29, 2016

As a more future-proof step, we could write a server-side method to run in autoform onSuccess, using the Validated Methods package. In general, we should be porting our allow/deny rules into Validated Methods.

@brylie
Copy link
Contributor

brylie commented Sep 29, 2016

This is a kind of tricky situation. On one hand, FileCollection package relies on collection allow/deny rules, so it may not be easy to create a ValidatedMethod for the FileCollection insert.

The alternative would be to create a ValidatedMethod where we add the user to the 'managers' role.

What other options do you see @marla-singer?

@brylie
Copy link
Contributor

brylie commented Sep 29, 2016

I created a quick PR to resolve this issue.

Instead of using the allow/deny approach for the Roles collection, I simply add the 'manager' role on startup. Then, when users create an API, they are given the 'manager' role.

@marla-singer
Copy link
Contributor

marla-singer commented Sep 29, 2016

I created a quick PR to resolve this issue.
Instead of using the allow/deny approach for the Roles collection, I simply add the 'manager' role on startup. Then, when users create an API, they are given the 'manager' role.

I think it's better solution at the moment

@brylie
Copy link
Contributor

brylie commented Sep 29, 2016

@marla-singer thanks. Will you please review the PR #1660?

@marla-singer
Copy link
Contributor

@brylie Yes. I'll also test if these fixes the problem with uploading a logo

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

No branches or pull requests

4 participants