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

Add outline for Authorization Cheat Sheet #401

Merged
merged 3 commits into from Jun 8, 2020
Merged

Add outline for Authorization Cheat Sheet #401

merged 3 commits into from Jun 8, 2020

Conversation

@KellyMarchewa
Copy link
Contributor

@KellyMarchewa KellyMarchewa commented May 31, 2020

This PR covers issue #394

Copy link
Contributor

@ThunderSon ThunderSon left a comment

Title and headers for the document should be concise and on point. No need to describe a whole point in a title.

cheatsheets_draft/Authorization_Cheat_Sheet.md Outdated Show resolved Hide resolved
cheatsheets_draft/Authorization_Cheat_Sheet.md Outdated Show resolved Hide resolved
cheatsheets_draft/Authorization_Cheat_Sheet.md Outdated Show resolved Hide resolved
cheatsheets_draft/Authorization_Cheat_Sheet.md Outdated Show resolved Hide resolved
cheatsheets_draft/Authorization_Cheat_Sheet.md Outdated Show resolved Hide resolved
cheatsheets_draft/Authorization_Cheat_Sheet.md Outdated Show resolved Hide resolved

Since, depending on context, the user probably should not be able to access all resources of type X, but only those are own or are otherwise privileged to perform operations on. That is, a simple role check may not be sufficient for all use cases; i.e. a user with role "customer" should not be able to access all "account" objects based on their role as a customer, but only their own account. Could perhaps be merged with recommendation on Feature/Attribute Based Access Control above?

### Ensure Lookup IDs are Not Accessible Even When Guessed or Cannot Be Tampered With

This comment has been minimized.

@ThunderSon

ThunderSon Jun 1, 2020
Contributor

Be sure to discuss that this might not be something that they would need. Sometimes users need to be able to find other users, or to have a certain pattern. e.g. forums, social apps, etc.
So it is important to mention in a small bit that based on the threat model (Banking app, accounts, admin profiles, locations, etc.) should follow stringent controls from obfuscating the ID to implementing strong controls on them.

Copy link
Contributor

@rbsec rbsec left a comment

This looks like a good outline, and certainly an improvement on the legacy CS. A few thoughts on stuff to add

  • Considerations able single-tenant vs multi-tenant applications.
  • Safely exiting if access control checks fail (i.e, not just sending a 302 header and continuing with execution like you often see in PHP).
  • Making sure that you're checking access control in the right place (e.g, the destination of the POST request, not the page with the <form>)
  • Enforcing authorisation on static resources where appropriate (for example, having raw PDFs or suchlike that can just be downloaded directly if the URL is known).

## Introduction

Focus on definition and distinguishing it from authentication. Perhaps explore types of privilege escalation (horizontal and vertical). Not sure if we should briefly explore the different access control models (RBAC, MAC, DAC, etc) or just leave it out?

This comment has been minimized.

@rbsec

rbsec Jun 1, 2020
Contributor

I think that introducing MAC is just adding complication - it's not relevant to 99% of developers.

This comment has been minimized.

@ThunderSon

ThunderSon Jun 1, 2020
Contributor

I am definitely against describing them. Maybe mention Access Controls in the references section, since in no way is this going to make it "intriguing" and actionable if we mention non-relevant stuff to the CS.

cheatsheets_draft/Authorization_Cheat_Sheet.md Outdated Show resolved Hide resolved
  * Removed excess headings and added 3 new ones.

  * Renamed/shortened headers.

  * Adjusted some comments/paragraphs in accordance with feedback.
@KellyMarchewa
Copy link
Contributor Author

@KellyMarchewa KellyMarchewa commented Jun 2, 2020

Thanks much for all the excellent feedback. I believe I have incorporated the feedback with the exception of single-tenant vs multi-tenant applications considerations. Is this something we want to create a dedicated subsection for or discuss where applicable in other sections?

Copy link
Contributor

@ThunderSon ThunderSon left a comment

LGTM

@ThunderSon ThunderSon requested a review from rbsec Jun 7, 2020
Copy link
Collaborator

@mackowski mackowski left a comment

Looks good to me!

@mackowski mackowski merged commit b6aa0b4 into OWASP:master Jun 8, 2020
2 of 3 checks passed
2 of 3 checks passed
link-check link-check
Details
lint
Details
Publishing Check
Details
@KellyMarchewa KellyMarchewa deleted the KellyMarchewa:auth-cs branch Jun 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants