Skip to content

Fix permission bugs in team API #647

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

Merged
merged 1 commit into from
Jan 20, 2017
Merged

Conversation

ethantkoenig
Copy link
Member

Some endpoints unnecessarily required the authenticated user to be an admin (basically everything removed from api/v1/admin/org_team.go).

Other endpoints didn't check that the authenticated user was an organization member.

@lunny lunny added this to the 1.1.0 milestone Jan 12, 2017
)

// ListTeams list all the teams of an organization
func ListTeams(ctx *context.APIContext) {
org := ctx.Org.Organization
if !org.IsOrgMember(ctx.User.ID) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This depends public or private status of org members. If public, then everyone could visit the members.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what you are saying; this is an endpoint for viewing an organization's teams, not for viewing the members of those teams.

Regardless, the corresponding endpoint in the Github API (GET /orgs/:orgname/teams) is only accessible to organization members, so I believe that is what we want to do here


// GetTeamMembers api for get a team's members
func GetTeamMembers(ctx *context.APIContext) {
if !models.IsOrganizationMember(ctx.Org.Team.OrgID, ctx.User.ID) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The corresponding Github endpoint (GET /teams/:id) is only accessible to organization members (even if there are public members of the team), so I believe that is what we want to do here.

@lunny lunny added modifies/api This PR adds API routes or modifies them type/bug labels Jan 12, 2017
@lunny
Copy link
Member

lunny commented Jan 14, 2017

LGTM

@tboerger tboerger added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Jan 14, 2017
@Bwko
Copy link
Member

Bwko commented Jan 19, 2017

LGTM

@tboerger tboerger added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jan 19, 2017
@lunny
Copy link
Member

lunny commented Jan 20, 2017

conflicted.

@appleboy
Copy link
Member

LGTM

@ethantkoenig
Copy link
Member Author

ethantkoenig commented Jan 20, 2017

Don't merge yet, there was something weird resolving the conflict

Fixed

@lunny lunny merged commit 74bbec3 into go-gitea:master Jan 20, 2017
Bwko added a commit to Bwko/gitea that referenced this pull request Jan 20, 2017
lunny pushed a commit that referenced this pull request Jan 20, 2017
@ethantkoenig ethantkoenig deleted the api/org_fixes branch January 22, 2017 17:19
@thomsh
Copy link

thomsh commented Jun 22, 2017

Hello,
@ethantkoenig with this old commit you remove two use full endpoints :

/admin/orgs/:orgname
/admin/teams/:teamid

Actually, in v1.1.2, there is no API endpoint to replace them.

If you didn't remove anything in the /admin/users endpoint,
there was no permission bug with orgs and teams endpoints ? Or not?
I'm just trying to understand ;)

Maybe I should open a issue about that ?

Have a nice day

@ethantkoenig
Copy link
Member Author

@theznx The POST /admin/orgs/:orgname/ endpoint was moved to POST /orgs/:orgname, and the PATCH /admin/teams/:teamid and DELETE /admin/teams/:teamid endpoints were moved to PATCH /teams/:teamid and DELETE /teams/:teamid respectively.

@thomsh
Copy link

thomsh commented Jun 23, 2017

@ethantkoenig Oh! Sorry for the inconvenience.

@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/api This PR adds API routes or modifies them type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants