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

Store permission don't work as in doc #95

Closed
maxwarch opened this issue Oct 12, 2016 · 8 comments
Closed

Store permission don't work as in doc #95

maxwarch opened this issue Oct 12, 2016 · 8 comments
Assignees
Labels

Comments

@maxwarch
Copy link

Hi,

i follow the doc to set permission to the store :

FichiersStore.setPermissions(new UploadFS.StorePermissions({
        insert: function (userId, doc) {
            return userId;
        },
        update: function (userId, doc) {
            return userId === doc.userId;
        },
        remove: function (userId, doc) {
            return userId === doc.userId;
        }
    }));

This work even when the user is not loggin.

So, to prevent user's upload when he's not login, i must do this

FichiersStore.setPermissions(new UploadFS.StorePermissions({
        insert: function (userId, doc) {
            if(!!userId === false) throw new Meteor.Error('Erreur upload', 'Vous n\'avez pas la permission');
        },
        update: function (userId, doc) {
            return userId === doc.userId;
        },
        remove: function (userId, doc) {
            return userId === doc.userId;
        }
    }));

This work and block the user.
Can you confirm this ?

Thanks for your support.

@jalik jalik closed this as completed in 4333d80 Oct 13, 2016
@maxwarch
Copy link
Author

Thanks, but i get another error in server when i upload using the permission :

insert: function (userId, doc) {
            return userId;
        },
update: function (userId, doc) {
            console.log(userId, doc); 
            return userId === doc.userId;
        },
Exception in callback of async function: Error: Forbidden [forbidden]
    at Object.<anonymous> (packages/jalik:ufs/ufs-store.js:395:23)
    at packages/matb33_collection-hooks/update.js:51:1
    at Array.forEach (native)
    at Function._.each._.forEach (packages/underscore/underscore.js:105:1)
    at packages/matb33_collection-hooks/update.js:50:1
    at Array.forEach (native)
    at Function._.each._.forEach (packages/underscore/underscore.js:105:1)
    at Object.<anonymous> (packages/matb33_collection-hooks/update.js:49:1)
    at Object.collection.(anonymous function) [as update] (packages/matb33_collection-hooks/
    at [object Object].update (packages/mongo/collection.js:589:29)

In fact, after search in js, userId becomes undefined during check update permission. I can't find why.

@jalik
Copy link
Owner

jalik commented Oct 13, 2016

@maxwarch, currently there is a problem I need to fix with the upload pattern design.
When UFS starts uploading, it creates the file in database using a Meteor method, so the userId is known at this moment, but while uploading it uses HTTP POST requests with only a token that replaces the userId since userId is not known outside of Meteor methods (Fiber), so the update permission is checked at this moment, thus it's why the userId is undefined.

Iam sorry I could not fix that yesterday but since there was really an issue with the permissions not checked I deployed a partial fix, I'll try to finish the fix ASAP.

You could replace your update check by this one as a temporary fix :

update: function (userId, doc) {
        // Allow updates only if user is unknown and doc uploading or user is owner
        return (!userId && !doc.complete) || userId === doc.userId;
 },

Merci de votre patience ;-)

@jalik jalik reopened this Oct 13, 2016
@maxwarch
Copy link
Author

Thank you, are you french ?? ;)
I remove the update permission to fix this.

@seabrus
Copy link

seabrus commented Oct 13, 2016

@maxwarch @jalik Hi, confirm the problem and would like to add some details, maybe they could be helpful.

I got absolutely the same error messages when trying to upload files. I use GridFSStore. A record about a file appears in the file collection (e.g., Images) but it doesn't appears in the GridFS collection db.uploadfs.files. The method transformWrite doesn't start too. The permissions are quite basic:

  permissions: new UploadFS.StorePermissions({
   ...
    update: function (userId, doc) {
      return (userId && userId === doc.userId);
    },
    ...
  }),

jalik added a commit that referenced this issue Oct 16, 2016
Fixes update permission check (#95)
@jalik
Copy link
Owner

jalik commented Oct 16, 2016

@maxwarch, je suis de Polynésie française, donc oui je suis Français ;)

@seabrus @maxwarch, I think the problem is now fixed, you can use permissions as normally, some permission checks were done because of the update hook on the collection in a context where the userId was not known.. so updates during the upload are not throwing exceptions anymore.

By the way I forgot to add the token verification during upload (my bad), it's now fixed ! so it's more secure than before (if you're concerned about this).

@jalik jalik closed this as completed Oct 16, 2016
@maxwarch
Copy link
Author

@jalik Ah d'accord ! J'image que tu tu ne dois pas être loin de la mer, pour travailler c'est plus sympa ;)
Thanks for your fix, the update works. But, i've got some weird errors when i upload.
Sometimes, i upload and Meteor seems to loose the connection with MongoDB, i must restart the server to work again. The client app works normally, but i've no more collection in mongol, even my Account and no server error, no client console error.
And the last error i've got, when i upload multiple files, i've got server error :
Exception while invoking method 'ufsCreate' MongoError: server instance pool was destroyed

I think that i don't have these errors before your first fix v0.7.0_2, i will try this version to be sure.

Sorry to reopen again this issue ;)
Thanks for your support.

@jalik
Copy link
Owner

jalik commented Oct 23, 2016

@maxwarch what about your last message ? can you confirm that it's related to UploadFS or GridFS store ?

@jalik jalik reopened this Oct 23, 2016
@maxwarch
Copy link
Author

Sorry for my late answer.
After tests, i think it's my router (i work with angular-meteor). I place this in the run block :

    $rootScope.$on('$locationChangeSuccess', function(event, toState, toParams, fromState, fromParams) {
        if(!us.isLoggedIn()){
            if(toState.indexOf('register') === -1) $state.go('login');
        }
    });

And when i comment these lines, it works without errors.
I must retry to confirm but i'm on another project for the moment.

Thanks.

@jalik jalik self-assigned this Jan 10, 2017
@jalik jalik added the bug label Jan 10, 2017
@jalik jalik closed this as completed Jan 10, 2017
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

3 participants