-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
compare/create PR produces a 404 #6302
Comments
My log which apparently is related to this issue and not #6225 : https://gist.github.com/ptman/f76b398d0503e5c50424c1ea8cbfafec . The permissions should be correct. It worked nicely with 1.7.2. How can I check? I haven't been able to figure out a URL that would work. I mostly end up on the 404 page by following the URL that's returned via git command line, but also create new pull request takes me to the same 404 URL. |
OK @ptman, I assume from your statements you're trying to try do a PR from a user's forked repository on to an organisation's private repository? Is the user a member of a team in the organisation and is that repository in that team? Does that team have permission to read pull requests? Could you check whether the PR url works if you happen to be an admin? Presumably the PR url is the same on v1.7.2 and v1.7.4? If not could you paste the differences? |
Yes, the URL is exactly the same. The user is an admin and owners team in organisation. Should have all permissions. |
Just check that a) the user is in the team b) the repository is in the team and c) that the team has permission to read pull requests. It's notable that there are two things that change between 1.7.2 and 1.7.4: the permission check, and possibly the URL. If you tell me that the URL is the same on 1.7.2 then it must be the permission check. |
Yes, I understood you the first time. My user is an admin according to site administration. There is only one team in the organisation. The default team called owners. My user is part of that team. That team has administrator access to the organisation. The URL is the very same. It's shown in the logs. I sanitized it by renaming the organization to org etc., but didn't change the structure otherwise |
Hi @ptman, Sorry I think I got distracted with a few other things. OK. So where did we get to?
The fact that it's 404 and #6098 is implicated places this at: You could confirm this by setting log to Trace. |
Ran into the same with 1.7.5 (did the very loooooooong upgrade from Gogs this weekend, using all versions in between). log level set to trace:
Will check the downgrade to 1.7.2 |
(Dirty) Downgrade to 1.7.2 worked.... (without DB restore from 1.7.2) |
aha Thank you @morph027 Now before you suggest staying on 1.7.2 - there is a major security bug in Gogs and Gitea earlier than 1.7.6 so I think we need to fix this. |
OK so here's a good test. Your user who can't create a pull on 1.7.5 - can they create comments on a pull that they did not post? Because that's the same test that is being used from 1.7.4. |
Hi! Thanks for your answer. Yes, just tried and i was able to comment on another users PR. |
Ok that's extremely odd. Here's the test involved (L1158): Lines 1152 to 1167 in c168095
That's very odd that that would succeed but this fails: Lines 701 to 707 in c168095
|
No this really doesn't make sense. Let's break down L1158: Line 1158 in c168095
Looking at the definition of ctx.Repo: Lines 34 to 56 in c168095
We see that Within Line 226 in c168095
And this is called from Line 336 in c168095
Which is a handler wrapped around This is a equivalent to I genuinely don't understand why if you can comment on a PR on the head repo you can't do the pull-request. |
Weird, of course. Comment on PR (which works):
Trying to create a PR:
|
BTW....as this was an migration from Gogs, we do have warnings (which should be safe to ignore as i've read on other issues):
But as this is totaly weird, probably there's something buried deep in it. |
@morph027 does that user have the permission of reading code on this repository? |
Yes, i can browse the web repo view and clone the repo using git. |
Ok I've put up a pr to 1.9 to heavily log.Trace all permissions and denials. It's #6618 One thing I've noticed looking at this code is that the wrong repo id is returned on that trace message you've got but that's by the by. If you are able to could you build this pr #6618, take a safe copy of your db and test again with trace on to get the permissions? On master you can target your logs so that all the trace logs don't appear eg: [log]
Mode=console,permissiontrace
Level=info
Redirect_macaron_log=true
Xorm=file
[log.permissiontrace]
Mode=console
Level=trace
Expression=permission Making it easier to see what you're looking for. |
Thanks, will try tonight (or probably spawn a test clone). |
Pulled 1.9-dev branch, applied the PR, built and tested: 404 when comparing
everyhing ok when commenting
(redacted the real values to my-user, organization and repository) |
Did you change the id numbers too? I actually needed those to compare against, oh and could I get the permission denied logs too. |
Now this bit is interesting:
Why is But if you look at the permissions that you're receiving it only has Code and Releases. I suspect it's a Team? I'd have to go back and add some more code to the PR I think to determine it though. Now the units returned here are interesting because they're precisely the reason the permissions is failing - it is testing if you can read UnitTypePullRequests. So is it possible adjust the team permissions? I'll add some more things to #6618 to hopefully figure out what the 824707102096:%!s(*interface {}=0xc0045fb570) could be. |
Hi! No, i did not change the ids. This happens to users in team, yes. |
I think this was missed in my earlier comment - your team doesn't appear to have the read pulls unit for the head repository. Could you check? It might be that you need to turn it off save and then put it back on in case this was an error in schema migration. Could you also try adding a new team to see if that new team has permission too? Many thanks |
Just created a a local test clone and tried to reproduce our git:
So there must be something inside the database regarding all the "old" (migrated) repos. I'll try to create a useful database diff between a non-working and this working repo. |
So the old repos do have something which is bogus. |
Deleting and re-forking the repos also make PRs work again. I can try to debug further (got multiple sql dumps to compare to each other), but for now, deleting and forking again looks like valid option (especially when using the API). |
What should I change for the repo (in the DB, using SQL) so that I don't have to remove the repo? |
Not sure, still trying to figure out. This is the diff after re-creating the fork:
|
The Interesting that the No changes in the @zeripath any useful hints in this diff? |
It's so strange. Create repository or fork repository will also insert defaultRepoUnits = []UnitType{
UnitTypeCode,
UnitTypeIssues,
UnitTypePullRequests,
UnitTypeReleases,
UnitTypeWiki,
} |
I have removed some features for my fork so I only have UnitTypeCode and UnitTypeReleases on my fork. But how could I check the permissions from the database? The fork was created ages ago. Was there previously some problem with setting the correct permissions on a fork? |
So that's maybe a bug, we should check the original repository's UnitTypePullRequest but not forked repository's. |
Yes, that's correct. By adding PRs to my fork, 404s disappear. |
fwiw, here's what the mysql server's query log shows when I open a
|
Awesome, just tested the PR and it looks like it's working 👏 ❤️ |
Note: Successfully upgraded to 1.8.1 🎉 Thank you! |
Yes, works for me as well, thanks! |
[x]
):Description
when trying to create a PR gitea shows a 404.
Happens both on
No relevant entries in the log files.
After downgrading to 1.7.2 everything works fine again.
Screenshots
The text was updated successfully, but these errors were encountered: