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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add Read/Write Permission #2550
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if !d.CheckReadPerm(r.URL.Path) { | ||
return http.StatusForbidden, nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think read permission should be checked here
http/data.go
Outdated
if !rule.Allow { | ||
write = false | ||
} else { | ||
write = strings.Contains(rule.Perm, "write") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if !rule.Allow { | |
write = false | |
} else { | |
write = strings.Contains(rule.Perm, "write") | |
} | |
if !rule.Allow { | |
write = false | |
continue | |
} | |
write = strings.Contains(rule.Perm, "write") |
http/data.go
Outdated
if !rule.Allow { | ||
write = false | ||
} else { | ||
write = strings.Contains(rule.Perm, "write") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if !rule.Allow { | |
write = false | |
} else { | |
write = strings.Contains(rule.Perm, "write") | |
} | |
if !rule.Allow { | |
write = false | |
continue | |
} | |
write = strings.Contains(rule.Perm, "write") |
type Perms struct { | ||
Read bool `json:"read"` | ||
Write bool `json:"write"` | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this struct is not used
const rule = this.rules[index]; | ||
const isRead = | ||
ruleName === "read" | ||
? rule.perm.includes("read") | ||
: !rule.perm.includes("read"); | ||
const isWrite = | ||
ruleName === "write" | ||
? rule.perm.includes("write") | ||
: !rule.perm.includes("write"); | ||
this.rules[index].perm = `${!isRead ? "read" : ""}${ | ||
!isRead && !isWrite ? "|" : "" | ||
}${!isWrite ? "write" : ""}`; | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not to store it as an inner object?
perms: {read: true, write: true}
} else { | ||
read = strings.Contains(rule.Perm, "read") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's follow the same code style as in CheckWritePerm
} else { | ||
read = strings.Contains(rule.Perm, "read") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
813c2e9
to
66dfbb3
Compare
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
This PR was closed because it has been stalled for 5 days with no activity. |
Description
Add Read/Write permission to filebrowser.
It works like ACL in linux
馃毃 Before submitting your PR, please read community, and indicate which issues (in any of the repos) are either fixed or closed by this PR. See GitHub Help: Closing issues using keywords.
Further comments
None.