You could disable the Signup from website settings.
I have done as a temporary security measure, but we would like to sell things online at some point (as well as actually using some custom portals that our suppliers are allowed to see).
Customers, suppliers, basically everyone who has access to the portal will only see documents that belong to them. For example, customer A will not see invoices or orders of customer B.
Also, if you’re thinking of selling items through the website, you will need the portal.
And, you can hide those unrelated portal pages from DocTypes. For example, Customize Form → Projects → uncheck “has web view”.
1 Like
A bit of brief testing suggests this is not the case. Yes, they don’t appear in the list, but plug the routes in directly and any supplier can see any purchase order on the system regardless of supplier…
If the routes were randomised this wouldn’t be a disaster but since they are just in numerical order, it would be trivial to valid documents.
Also there is no ‘web view’ under Customize Form for me (I am on V12).
There is one on ‘Doctype’ but it isn’t turned on anyway (for Project or Purchase Order).
This report would seem to be somewhat similar issue, but an internal violation case Fields with User Permission · Issue #98 · frappe/hrms · GitHub
It gets weirder: seems you can’t see any PO at all if you have the ‘Customer’ role (or more specifically, if you have a Customer attached to the contact attached to the user). I think the controllers → website_list_for_contact → has_website_permission is buggy and broken.
Time to do some debugging…
I still think you shouldn’t be able to see any documents you don’t have role-based permissions for (which would simply require adding ‘doc.has_permission(‘read’)’ to templates/pages/order.py, as is done for other pages).
How is it supposed to work? There are competing things here.
First, you want users with Desk access and Role permissions for (for example) Purchase Order to be able to see those on the website. This can be handled by role permissions: if you have the role, you have the permission.
Second, you might want website users to be able to see none of the documents, all of the documents, or just only those documents they are linked to. You can’t easily have all three options available, and I can’t quite work out how it is supposed to work in ERPNext.
If you require the user to have Role permissions to view even linked contacts, then you can easily restrict website users by just removing the permissions from ‘Supplier’. But then Desk viewers can’t view the documents either unless they are linked directly…
If you don’t require the user to have Role permissions to view their documents, then it is really hard to stop them. I think it can be done by adding a custom has_website_permission hook for each document type you want to restrict access to, but this is hacky and it should just be an option somewhere.
You might also just decide not to give users linked to Suppliers the Supplier role to prevent them . Unfortunately, you can’t easily do that as it is automatically assigned to them when they sign up.
I would note that the way I think it is currently working in ERPNext (or will be once an incidental bug is fixed) is that the Supplier role has permissions for Purchase Order, and as long as the user is linked by contact that is the only PO they will be able to view. But if that got deleted somehow, the system would fall back to role-based permission, and then they would be able to see all of them… Equally a system user linked to a Purchase Order would only be able to see their own POs!
Security issues can be reported here.
I already have (and have done in the past), but response is always glacially slow (reported another one about 2/3 weeks ago that I still haven’t heard back about yet)…
We need this fixed now, so was going to try and do it myself.
As I understand it, Role Permissions are for desk and api access. Portal access is handled differently, where users are able to see only documents linked to them.
EDIT: Investigating a bit more…
Pull request for a very minor issue that doesn’t fix all the problems (for example, a Supplier who is also a Customer can’t see their Supplier docs).
https://github.com/frappe/erpnext/pull/21827
I have also (coincidentally) realised that Portal Settings is broken because it was using ‘grid.only_sortable’ in the JS. In V11 this didn’t make the grid rows themselves read-only (or maybe just checkboxes?’ but in V12 this seems to make the rows read-only as well. Which is annoying, as that is functionality I have also used… The aim there is to prevent rows being added or deleted, but allowing some alteration of values in columns that aren’t read-only (i.e. turning things on and off).
1 Like
I think something’s amiss with your particular instance. On my install, also v12, I’m seeing very different behavior:
- Portal settings are fully editable, and they allow me to control which kinds of documents are visible from the portal.
- Website users are only able to see documents that they are linked to, and they get a permission error if they manually enter urls routes for documents they shouldn’t see.
I can confirm that Purchase Orders are unexpectedly NOT visible for users who are both customers and suppliers. I am also able to create a Purchase Invoice from a Purchase Order. This appears to be intentional, however, and I don’t know enough about accounting to have a sense of why this choice was made.
Most broadly, I am also concerned by the unexpected behavior whereby users are matched via email address. I think it would be a far better approach to require manual linking, though I’m not encountering any of the serious security issues you are.
Edit: Apologies, it seems we’re posting at the same time, and you’ve addressed a few things here already 
At least I’m not going completely mad 
What version of V12 (commit ref or whatever) are you using? I’m going to do a bench update and see if the only_sortable/Portal Settings thing persists, and if its broken in the newest version try and track down the offending commit…
Actually I suspect its related to this commit:
https://github.com/frappe/frappe/commit/4b2dc1a336aea6d3ff196b2780721d70605a35de
Edit: updating the bench does fix the Portal Settings issue.
1 Like
Good find! The version I tested was a local instance, and I’m away from that computer now unfortunately so I can’t check the exact version. Based on when I set it up, I suspect it was the 12.7.1 release (though it could have been slightly earlier). I did have develop mode on, though, so it’s possible I changed something.
Anyway, I’ll keep poking at it, and I’m glad you’re checking into the security here. We had been thinking about rolling out the portal ourselves, and you’ve raised a few important questions.
The more you look at the code (which incidentally doesn’t have a single comment in the entire file) the worse it gets…
As well as completely failing the account for a user who is both a Customer and a Supplier (or more specifically, has any Customer linked to their Contact as well as any Supplier linked to their contact), the bit of code that matches contacts to the user looks at the contact email_id, rather than the ‘user’ field. So you might think that emptying the ‘User’ field on the contact (which is normally auto-filled with a matching User) would remove the permissions access. But you would be wrong…
1 Like
Well, that is precisely the point of insisting and promoting ‘the tests’ as the truth - to state and enforce the agreed convention and understanding of how the code is supposed to work:
I agree entirely - so if I can work out how it is supposed to work, I may try and create some access tests… This will require either Cypress tests or just using requests to check http status (which I have used before in my custom app for checking access controls).
1 Like
On a positive note, speaking of tests these automated ones that run constantly are encouraging and reassuring - the current counts are
772 erpenxt tests here
446 frappe tests here
These counts below are a bit higher, from ‘do nothing’ test cases I think:
frappe@ubuntu1804lts:~/frappe-bench$ find apps/erpnext -name ‘test*.py’ | xargs grep ‘def test*’ | wc
799 2407 93304
frappe@ubuntu1804lts:~/frappe-bench$ find apps/frappe -name ‘test*.py’ | xargs grep ‘def test*’ | wc
488 1473 44618