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

[API] Delete Yourself #19439

Open
6543 opened this issue Apr 20, 2022 · 17 comments
Open

[API] Delete Yourself #19439

6543 opened this issue Apr 20, 2022 · 17 comments
Labels
modifies/api This PR adds API routes or modifies them type/feature Completely new functionality. Can only be merged if feature freeze is not active.

Comments

@6543
Copy link
Member

6543 commented Apr 20, 2022

the user should have an api to delete his own account

you just have to look at

if err := user_service.DeleteUser(ctx.ContextUser); err != nil {
if models.IsErrUserOwnRepos(err) ||
models.IsErrUserHasOrgs(err) ||
models.IsErrUserOwnPackages(err) {
ctx.Error(http.StatusUnprocessableEntity, "", err)
} else {
ctx.Error(http.StatusInternalServerError, "DeleteUser", err)
}
return
}
log.Trace("Account deleted by admin(%s): %s", ctx.Doer.Name, ctx.ContextUser.Name)
ctx.Status(http.StatusNoContent)
}

@6543 6543 added type/feature Completely new functionality. Can only be merged if feature freeze is not active. modifies/api This PR adds API routes or modifies them good first issue Likely to be an easy fix labels Apr 20, 2022
@6543
Copy link
Member Author

6543 commented Apr 20, 2022

I would propose DELETE /user

and add some "save" mechansim like a option in body to say YES ... ?

@Gusted
Copy link
Contributor

Gusted commented Apr 21, 2022

What's the point of a save mechanism?

The API then will need two requests from those who utilize it.
Those that use the API as a front-end wrapper, such as GitNex, could display a "Are you sure" screen for this action ahead of time and avoid having to deal with our save mechanism to show such a screen to the user.

Most use-cases for this API call, will just make the two requests unconditionally, which, if you ask me, is a waste of resources.

@6543
Copy link
Member Author

6543 commented Apr 21, 2022

also an opinion ...

@jolheiser
Copy link
Member

I agree with @Gusted, a raw API has no need for a confirmation. That is a front-end job.

@6543
Copy link
Member Author

6543 commented Apr 21, 2022

ok so let it be one request to let them delete all ;)

@techknowlogick techknowlogick changed the title [API] Delete Yourselve [API] Delete Yourself Apr 21, 2022
@mscherer
Copy link
Contributor

In fact, wouldn't it be better to have the API mark the user "to be deleted" and delete after X weeks ? This would make revert easier, in case someone account is compromised.

@6543
Copy link
Member Author

6543 commented Apr 29, 2022

good idea, will require extra work for ALL delete and GET fuctions from models, so i thin it's a own issue (and worth it's own pull)

@dhruvmanila
Copy link
Contributor

This would also delete the admin user or should that be not allowed?

@wxiaoguang

This comment was marked as outdated.

@6543
Copy link
Member Author

6543 commented Sep 29, 2022

why they need to do so

alternative frontends

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Sep 29, 2022

why they need to do so

alternative frontends

For which case? Why? Is there anybody would delete their GitHub account by API?

@6543
Copy link
Member Author

6543 commented Sep 29, 2022

sorry I still dont understand why you are so frighten about such an api ?

@wxiaoguang

This comment was marked as outdated.

@wxiaoguang

This comment was marked as outdated.

@wxiaoguang

This comment was marked as off-topic.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Sep 29, 2022

sorry I still dont understand why you are so frighten about such an api ?

Sorry that I misunderstood some details, I see the latest reqBasicOrRevProxyAuth change.

@wxiaoguang
Copy link
Contributor

After reading the code reqBasicOrRevProxyAuth, I have more questions now ....

In the reqBasicOrRevProxyAuth, if the AuthedMethod is ReverseProxy, then the IsBasicAuth and CheckForOTP will be skipped.

Imagine that some users deploy private Gitea instance with ReverseProxy auth mode, does it mean that with this PR, other users can use the token to purge their account?

When many options come together, the problem becomes more complex (back to my first feeling, could the existing web page for "Delete Acount" be enough for daily usage?)

@wxiaoguang wxiaoguang removed the good first issue Likely to be an easy fix label Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
modifies/api This PR adds API routes or modifies them type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants