Fix api releases attachments wrong URL

[lunny]

Fix #26165
The problem should be fixed by #25639 but unfortunately it's not. Because custom URLs have been filled into attachments.

[JakobDev]

This throws the nice URL completely from the API, which causes problems for people who want this. So you broke one feature to fix another. It think it's way better to just add add a new field e.g. api_download_url which contains the URL that can be used with API Authentication.

Dismiss

[JakobDev]

I found something interesting: It looks like the route {repo}releases/download/{release}/{filename} is public. SO you can download from private repos without a Authetification. But this also means that people who should not allowed to do this can also download files. As this route can be guessed by humans, this could be a problem in theory.

[JakobDev]

Forgot was I wrote in the last post. I was stupid and didn't notice that I was also logged in in my second Browser.

But I found something more interesting: {repo}releases/download/{release}/{filename} does not work with an API Key, but /attachments/00d77555-e1ea-4839-94d0-49a6213bbf53 does. If I open /attachments/{uuid} from my private Repo, without logging in, it does gives me a 404. But if I open attachments/{uuid}?token={token} it does work. Maybe this can be fixed without changing the API. I try to investigate this.

[lunny]

I don't know what did you mean the break? The previous attachment URL does not work if it's a private repository.

[lunny]

Forgot was I wrote in the last post. I was stupid and didn't notice that I was also logged in in my second Browser.

But I found something more interesting: {repo}releases/download/{release}/{filename} does not work with an API Key, but /attachments/00d77555-e1ea-4839-94d0-49a6213bbf53 does. If I open /attachments/{uuid} from my private Repo, without logging in, it does gives me a 404. But if I open attachments/{uuid}?token={token} it does work. Maybe this can be fixed without changing the API. I try to investigate this.

/attachments/00d77555-e1ea-4839-94d0-49a6213bbf53 should also ignore the token imo.

[JakobDev]

should also ignore the token imo.

It does not ignore the token. You can test it yourself. I'm working to find out more in the moment.

[lunny]

should also ignore the token imo.

It does not ignore the token. You can test it yourself. I'm working to find out more in the moment.

Yes, it's not currently, but I just meant maybe it should ignore.
On basic.go:46

	if !middleware.IsAPIPath(req) && !isContainerPath(req) && !isAttachmentDownload(req) && !isGitRawReleaseOrLFSPath(req) {
		return nil, nil
	}

containers, attachment download, git http protocol or lfs will support PAT currently.

[JakobDev]

Yes, it's not currently, but I just meant maybe it should ignore.

Why should be do that? This can break existing scripts. We currently have a working solution, so we just needed to add the {repo}releases/download/{release}/{filename} route to isAttachmentDownload and everyone is happy.

[Gr3q]

Forgot was I wrote in the last post. I was stupid and didn't notice that I was also logged in in my second Browser.
But I found something more interesting: {repo}releases/download/{release}/{filename} does not work with an API Key, but /attachments/00d77555-e1ea-4839-94d0-49a6213bbf53 does. If I open /attachments/{uuid} from my private Repo, without logging in, it does gives me a 404. But if I open attachments/{uuid}?token={token} it does work. Maybe this can be fixed without changing the API. I try to investigate this.

/attachments/00d77555-e1ea-4839-94d0-49a6213bbf53 should also ignore the token imo.

How am I going to download a release attachment from the cli via the API from private repos in that case? Am I forced to make the repo public?

[lunny]

Yes, it's not currently, but I just meant maybe it should ignore.

Why should be do that? This can break existing scripts. We currently have a working solution, so we just needed to add the {repo}releases/download/{release}/{filename} route to isAttachmentDownload and everyone is happy.

The benefit is users could run a API-only Gitea.

[JakobDev]

The benefit is users could run a API-only Gitea.

That's understandable, but we could support both. We add {repo}releases/download/{release}/{filename} to isAttachmentDownload. The API could return 2 values: browser_download_url, which returns the URL seen in the Browser. There are people (like me) who want the API return the same values as the browser Website. And another value called browser_download_url which returns the value from the API route. So everyone will be happy.

[CMiksche]

And another value called browser_download_url which returns the value from the API route. So everyone will be happy.

I think you mean api_download_url?

[johanvdw]

The benefit is users could run a API-only Gitea.

That's understandable, but we could support both. We add {repo}releases/download/{release}/{filename} to isAttachmentDownload. The API could return 2 values: browser_download_url, which returns the URL seen in the Browser.

I don't think it is a good idea to return a url from the api if you can not access it using the same method used in the api. That will only lead to confusion.

[JakobDev]

I think you mean api_download_url?

Yes

I don't think it is a good idea to return a url from the api if you can not access it using the same method used in the api.

We currently have Web routes e.g. /attachments/{uuid} which allows also API auth. Why not the Opposite? We allow Web auth for some API routes and show the link to the files on the API route on the Frontend.

[lunny]

I think you mean api_download_url?

Yes

I don't think it is a good idea to return a url from the api if you can not access it using the same method used in the api.

We currently have Web routes e.g. /attachments/{uuid} which allows also API auth. Why not the Opposite? We allow Web auth for some API routes and show the link to the files on the API route on the Frontend.

Since CustomURL cannot be visited with token but api/repos/user2/repo1/releases/1/assets/9 can. This PR just change the URL to the later. What's wrong with the PR? @JakobDev @wxiaoguang

[JakobDev]

The new API route uses the IDs instead of names. So here are 2 URLs:

user1/user2/v1.2/LinuxPortable.zip
api/repos/user2/repo1/releases/1/assets/9

The first one is the current one. I can tell by just looking at the URL that I will the Linux build of Version 1.2 of the Software. Can I get the same Information with the Second URL?

We now have 2 different routes that lead to the same file (I'm generally not an fan of this). You can query the new API route through the API, but you can no longer query the route that is shown in the Browser. There are maybe situations where you need this. e.g. You want to automagically publish a Link on your site for your Nightly build. But your Nightly builds are only available for Patreon, which are logged in on the Gitea instance. So the Link to the Web URL works, but the Link to the API URL will not work, since the API uses different auth. There are also some people who may find it suspicious, when they see another Link in a Install script than they see in the Web Interface. IMHO a API should alwayas give you all Information that you also see in the Web Interface.

I know that the Web route does not work with a token, but for public repos this does not matter. If you don't use release downloads from private repos through the API, you will get a degraded experience with no benefits with this PR.

We currently support API identification for /attachments/<uuid> URL although this is handled by the Web route. I think the best Way to solve such problems in the Future is to add a third route for files next to the Web and API route (e.g. /downloads), which supports Web and API auth methods, so web users and API users can use the same link, which can avoid confusion. If we add a API only mode later which deactivates the Web route, the downloads route will still be active.

[lunny]

1 Even this PR will do not use api/repos/user2/repo1/releases/1/assets/9, it will still be there except we introduce api v2
2 I think if users want to use API, it almost will be used by a script or robots. Nobody will care about reading the information from the route because the attachment information is nearby, they don't need to guess from the route path.

But I think your idea about moving file read out of web routes and api routes is a good idea. It should also include git Smart HTTP routes.

[JakobDev]

Even this PR will do not use api/repos/user2/repo1/releases/1/assets/9, it will still be there except we introduce api v2

For me, it looks like if this PR changes the API response to this format

I think if users want to use API, it almost will be used by a script or robot

For the API itself this is true. But the Bot might just be updating a download script where the Users cares about the URL.

[johanvdw]

I'm in favor of merging this PR. The new nice URL's break existing scripts because authentication no longer works. I think the discussion of changing the url's is interesting and good, but we should first make sure that the api can be used as before.

[Gr3q]

Even when this PR is merged I will need an API call to get this URL, I won't be able to get it via the UI.

[JakobDev]

As we currently have no API only Gitea, I think it's the best to just allow API auth for {repo}releases/download/{release}/{filename} like it's done for /attachments/{uuid} as a hotfix and to not break things. We can add a new new route like I suggested later on. I can start working on this by the end of the month, if you want me to do that.

[lunny]

As we currently have no API only Gitea, I think it's the best to just allow API auth for {repo}releases/download/{release}/{filename} like it's done for /attachments/{uuid} as a hotfix and to not break things. We can add a new new route like I suggested later on. I can start working on this by the end of the month, if you want me to do that.

I don't think this PR break anything. The removal of if attach.CustomDownloadURL != "" { lines doesn't affect any UI.
But will fix the API auth errors.

APIAssetDownloadURL

This PR is just a bugfix and with no any break. If you would like CustomDownloadURL could be visited from token, that should be another PR.

[JakobDev]

This PR is just a bugfix and with no any break

Not really.

Before CustomDownloadURL was added, we had the following case.
The URL can be used with Web Auth
The URL can be used with API Auth

We currently have the following case:
The URL can be used with Web Auth
The URL can't be used with API Auth

After your PR we will have the following case:
The URL can't be used with Web Auth
The URL can be used with API Auth

So you obviously break Web Auth to fix API Auth. We need to make it work with both.

Especially for Releases, the API might be used by a bot who post the Link somewhere where it can be downloaded when you are logged in.

Your new URL also still don't contain the Filename. There are People (including me) who Release and Filename as part of the URL, but that should be a easy fix.

[lunny]

Maybe you have a misunderstand. I don't think this will break Web Auth of the URL.

[JakobDev]

I understand that, but the URL the API returns will not support Web Auth.

[lunny]

Close as this PR is totally wrong. #26165's problem is basic auth is OK but token auth is not.