Add InternalTokenURI to load InternalToken from an external file

[techknowlogick]

(Continuing #4412)

URI HAVE TO start with 'file:' or 'file://'. Possibility to add http/s3 support in the future.
On errors it WILL fail hard (in case of misconfiguration OR the storage not being available)
The file WILL be read if it exists and the token extracted from it.
The file WILL be created if it does not exist.

Closes #3246

[techknowlogick]

This is here for the sake of linter failing due to not having a return, however the above switch case will either return success, or fatal error before reaching this line.

[codecov-io]

Codecov Report

Merging #5812 into master will decrease coverage by 0.01%.
The diff coverage is 17.24%.

@@            Coverage Diff            @@
##           master   #5812      +/-   ##
=========================================
- Coverage   38.82%   38.8%   -0.02%     
=========================================
  Files         359     359              
  Lines       51137   51175      +38     
=========================================
+ Hits        19854   19861       +7     
- Misses      28411   28442      +31     
  Partials     2872    2872

Impacted Files	Coverage Δ
modules/setting/setting.go	45.55% <17.24%> (-1.46%)	⬇️
models/repo_list.go	66.84% <0%> (-1.06%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 91775c1...8bfd48f. Read the comment docs.

[zeripath]

Why don't you write the internal token as an ini file - so you read in the initial file adjust the value of internal token and write it back out.

Then, if Uri is not a fully specified URL assume it's file. You should probably absolute that in the same way we do for app.ini at present. Finally if the Uri is nil set it to the current app.ini

That way you can lead Uri nil and it will assume Uri is the app.ini file and continue the current behaviour?

[zeripath]

Sorry fat fingers!

[lunny]

This PR just a refactor to load/save InteralToken and it will still write content to ini file so I don't think it will fix #3246 .

[techknowlogick]

@lunny With this PR the only time it will try to write to app.ini is if URL is invalid, and token is not set.

Edit: Also the ticket is a feature request to move token out of app.ini which this PR allows for

[techknowlogick]

resolved conflicts due to settings module split

[sapk]

Is this still needed since #3531 ? As we provide a simple way to generate valid token to generate a config file.

[techknowlogick]

@sapk yes, as this provides an enhancement so that we can store the secret separately (in docker image we could even use docker secret)

[sapk]

@techknowlogick docker secret are ready only if I remember well so if the it re-write it will still not work. And they are other key to keep secret on the config file. I think that it is a good PR but it is not the good problem.
The real problem is that the config file should not be changed if it already have a valid token.

[techknowlogick]

@sapk in the existing code (and also in PR) it should only write a token to configure ‘if len(token) == 0’ and I am unable to replicate having Gitea write to config if key is already set. This is why I have this set as enhancement rather than bug fix. If this gets merged in, then I can make future enhancement for other secrets in app.ini to read from external sources.

[lunny]

I think store the key to s3 or other place is not a good idea. But if we can support docker secrets or similar place will be better.

[techknowlogick]

@lunny this works with docker secrets right now, and maybe in future we add vault support.

[jolheiser]

This worked for me in testing.

One small thing to note @techknowlogick:

The file WILL be created if it does not exist.

Currently the file is not created if it doesn't exist, instead Gitea stops with a Fatal. Looking at the code, that seems intentional, so I'll approve. Just wanted to note it for clarification.

gitea/modules/setting/setting.go

Lines 1260 to 1282 in f666bbc

	fp, err := os.OpenFile(tempURI.RequestURI(), os.O_RDWR, 0600)
	if err != nil {
	log.Fatal(4, "Failed to open InternalTokenURI (%s): %v", uri, err)
	}
	defer fp.Close()
	buf, err := ioutil.ReadAll(fp)
	if err != nil {
	log.Fatal(4, "Failed to read InternalTokenURI (%s): %v", uri, err)
	}
	// No token in the file, generate one and store it.
	if len(buf) == 0 {
	token, err := generate.NewInternalToken()
	if err != nil {
	log.Fatal(4, "Error generate internal token: %v", err)
	}
	if _, err := io.WriteString(fp, token); err != nil {
	log.Fatal(4, "Error writing to InternalTokenURI (%s): %v", uri, err)
	}
	return token
	}
	return string(buf)

[techknowlogick]

@jolheiser yes, that is intentional. Thanks for the review :)

[cdrage]

@techknowlogick Any way we can add documentation too? As of right now, I believe this is an undocumented feature?

[techknowlogick]

@cdrage Just noticed this comment, sorry. I've added documentation in this PR #7234