fix: Read windows path error - 2 #557

PatrLind · 2020-10-15T09:41:48Z

Related PR: #514

I asked in the Slack chat if I should make this PR and it seems like @zepatrik and @jfcurran gave their permission with an emoji.

Proposed changes

I had the same issue as the person creating PR 514. That file:// URLs wasn't working correctly under Windows.
I looked into it a bit and it seems like the original author of the PR and also Ory have misunderstood how file:// URLs work.
I have added a helper function that can extract the file path from the URL for as many situations I can think of.
I have also added a few non standard ways (relative paths and incomplete URLs) to deal with file:// URLs because those were used in the original tests from Ory.

Checklist

I have read the contributing guidelines.
I have read the security policy.
I confirm that this pull request does not address a security
vulnerability. If this pull request addresses a security. vulnerability, I
confirm that I got green light (please contact
[email protected]) from the maintainers to push
the changes.
I have added tests that prove my fix is effective or that my feature
works.
I have added or changed the documentation.

Further comments

The tests didn't work on Windows before my fix, but now they work except for a test called TestFetcherWatchRepositoryFromKubernetesConfigMap. I am not sure how to make this test succeed since it seems to be written for non windows systems only. I have also run the tests under WSL (Linux) and they all pass, so I think this change should not cause any regressions.

I don't particularly like all the workarounds in the helper function, but it seems like it is common for people to write incorrect file:// URLs so I think they are needed.

…ersion

CLAassistant · 2020-10-15T09:41:54Z

All committers have signed the CLA.

zepatrik · 2020-10-15T09:47:45Z

The kubernetes test does not have to pass as you would not run kubernetes on windows. They do some symlink magic there to update files atomically, that's why we have that special case.

zepatrik

It would actually be very nice if you could add a CIrcleCI job or GitHub workflow to run the tests on windows. I think it would be enough to run go test ./.... If you have no experience with that, let me know.

helper/fileurl_test.go

helper/fileurl.go

PatrLind · 2020-10-15T10:14:11Z

I don't have any CIrcleCI or GitHub Workflow experience at all, but I can give it a try.

PatrLind · 2020-10-19T09:43:22Z

Ok, So I was not successful in doing a Windows CircleCI test, not sure how to do it. Sorry.
Also, how come the existing CircleCI tests are failing?

zepatrik · 2020-10-19T09:45:35Z

The error output is:

panic: proxy: expected 200: {"error":{"code":404,"status":"Not Found","message":"The requested resource could not be found"}}


goroutine 1 [running]:
main.main()
	/go/src/github.com/ory/oathkeeper/test/e2e/okclient/main.go:57 +0x22d
+ finish
+ cat ./oathkeeper.e2e.log
time=2020-10-15T09:52:52Z level=info msg=Config file loaded successfully. audience=application path=./config.yml service_name=oathkeeper service_version=
Thank you for using ORY Oathkeeper master!

Take security seriously and subscribe to the ORY Security Newsletter. Stay on top of new patches and security insights.                                                                                                

>> Subscribe now: http://eepurl.com/di390P <<
time=2020-10-15T09:52:52Z level=debug msg=Viper detected a configuration change, updating matching strategy audience=application event=matching_strategy_config_change service_name=ORY Oathkeeper service_version=master source=entrypoint
time=2020-10-15T09:52:52Z level=debug msg=Viper detected a configuration change, reloading config. audience=application event=config_change service_name=ORY Oathkeeper service_version=master source=entrypoint
time=2020-10-15T09:52:52Z level=debug msg=One or more access rule repositories changed, reloading access rules. audience=application event=repository_change file=file://e2e-rules.json service_name=ORY Oathkeeper service_version=master source=config_update
time=2020-10-15T09:52:52Z level=debug msg=Fetching access rules from given location because something changed. audience=application location=file://e2e-rules.json service_name=ORY Oathkeeper service_version=master
time=2020-10-15T09:52:52Z level=error msg=Unable to update access rules from given location, changes will be ignored. Check the configuration or restart the service if the issue persists. audience=application error=map[message:error converting YAML to JSON: yaml: did not find expected alphabetic or numeric character] file=file://e2e-rules.json service_name=ORY Oathkeeper service_version=master
time=2020-10-15T09:52:52Z level=info msg=Software quality assurance features are enabled. Learn more at: https://www.ory.sh/docs/ecosystem/sqa audience=application service_name=ORY Oathkeeper service_version=master
time=2020-10-15T09:52:52Z level=info msg=No tracer configured - skipping tracing setup audience=application service_name=ORY Oathkeeper service_version=master
time=2020-10-15T09:52:52Z level=info msg=Listening on http://:9000 audience=application service_name=ORY Oathkeeper service_version=master
time=2020-10-15T09:52:52Z level=info msg=TLS has not been configured for proxy, skipping audience=application service_name=ORY Oathkeeper service_version=master
time=2020-10-15T09:52:52Z level=info msg=TLS has not been configured for api, skipping audience=application service_name=ORY Oathkeeper service_version=master
time=2020-10-15T09:52:52Z level=info msg=Listening on http://:6661 audience=application service_name=ORY Oathkeeper service_version=master
time=2020-10-15T09:52:52Z level=info msg=Listening on http://:6660 audience=application service_name=ORY Oathkeeper service_version=master
time=2020-10-15T09:52:53Z level=info msg=started handling request http_request=map[headers:map[accept-encoding:gzip authorization:Value is sensitive and has been redacted. To see the value set config key "log.leak_sensitive_values = true" or environment variable "LOG_LEAK_SENSITIVE_VALUES=true". user-agent:Go-http-client/1.1] host:127.0.0.1:6660 method:GET path:/jwt query:<nil> remote:127.0.0.1:58560 scheme:http]
time=2020-10-15T09:52:53Z level=warning msg=Access request denied audience=application error=map[debug: message:Requested url does not match any rules reason: status:Not Found status_code:404] granted=false http_host=127.0.0.1:6660 http_method=GET http_url=http://127.0.0.1:6660/jwt http_user_agent=Go-http-client/1.1 service_name=ORY Oathkeeper service_version=master
time=2020-10-15T09:52:53Z level=error msg=An error occurred while handling a request code=404 debug= details=map[] error=The requested resource could not be found reason= request-id= status=404 writer=JSON
time=2020-10-15T09:52:53Z level=info msg=completed handling request http_request=map[headers:map[accept-encoding:gzip authorization:Value is sensitive and has been redacted. To see the value set config key "log.leak_sensitive_values = true" or environment variable "LOG_LEAK_SENSITIVE_VALUES=true". user-agent:Go-http-client/1.1] host:127.0.0.1:6660 method:GET path:/jwt query:<nil> remote:127.0.0.1:58560 scheme:http] http_response=map[status:404 text_status:Not Found took:2.852785ms]
+ echo -----
-----
+ cat ./api.e2e.log

PatrLind · 2020-10-19T10:18:45Z

Ok, I should have been more clear, I can see the logs but I am not sure what part of the output is the error that fails the test.
There are three errors, none of which seems to be related to what I did:

panic: proxy: expected 200: {"error":{"code":404,"status":"Not Found","message":"The requested resource could not be found"}}
level=error msg=Unable to update access rules from given location, changes will be ignored. Check the configuration or restart the service if the issue persists. audience=application error=map[message:error converting YAML to JSON: yaml: did not find expected alphabetic or numeric character] file=file://e2e-rules.json service_name=ORY Oathkeeper service_version=master
level=error msg=An error occurred while handling a request code=404 debug= details=map[] error=The requested resource could not be found reason= request-id= status=404 writer=JSON

All the relevant test seems to pass when I run them locally, but this log doesn't really run those tests at all. Maybe I need to run some other test script locally to find out where the issue is? As I am not so familiar with your build/test system some help here would be appreciated.

Edit:
Ok, So I think I understand why the test failed, some request returned a 404 that in turn caused a panic somewhere in the code.
The problem is that there is no information on where the error happened, so quite difficult to find. I guess I need to try to replicate all the HTTP accesses and see if I get anything similar.

Edit2:
Ok, sorry for the confusion, I think I understand how the tests are run now. I will try to fix it.

…ndard variants of file URLs

PatrLind · 2020-10-20T03:01:41Z

Please note that I have made some small change to rule/rule_test.go that corrected an issue that made the tests (or was it linting?) fail. I am not sure this change is allowed or should be in here. Should I remove it?
7635fbd

ci: windows unit test

The PatseURL function will handle parsing URLs with special handling of file:// URLs The updated GetURLFilePath function will return a correct path baed on the updated URL parsing.

zepatrik

Can you re-request a review when your done? Thx 😉

x/fileurl.go

zepatrik · 2020-10-22T08:56:53Z

x/fileurl.go

+
+// ParseURL parses rawurl into a URL structure with special handling for file:// URLs
+func ParseURL(rawurl string) (*url.URL, error) {
+	lcRawurl := strings.ToLower(rawurl)


rename

Suggested change

lcRawurl := strings.ToLower(rawurl)

lcRawURL := strings.ToLower(rawurl)

Wouldn't it be less error prone to do

Suggested change

lcRawurl := strings.ToLower(rawurl)

if runtime.GOOS == "windows" {

rawURL = strings.ToLower(rawurl)

}

and avoid the decision what variable to use for what check?

you are lower casing rawURL there because windows filepaths are case insensitive right? But they are case sensitive on POSIX
That was what I was referring to.

Not sure exactly what you mean here, the lcRawURL is only used to compare the initial part if the rawURL string so see if it starts with file:// (or any upper/lower case version of it).

Ah ok, could you then maybe use strings.Split(rawURL, "://") to get the scheme and use that for the checks?

At this point in the code I am first looking for file:/// (three slashes), if that is the case I am parsing it as a normal URL with an early exit. If that is not the case and it instead starts with file:// (two slashes) we trim the first 7 chars off the beginning and then continues the function. The lower case comparison has nothing to do with Windows, it just makes sure that FILE:// and file:// are treated the same.
I can change it to a regexp case insensitive comparison if that feels better.

zepatrik · 2020-10-22T09:15:51Z

x/fileurl.go

+		if len(parts) > 2 {
+			host = parts[2]
+		}
+		p := "/"
+		if len(parts) > 4 {
+			p += strings.Join(parts[3:], "/")
+		}


This took me quite some time to understand this part. Can you maybe rewrite it a bit? Or add comments? It is especially not clear why parts[3] is discarded when len(parts) == 4.

I have reworked this part now

zepatrik · 2020-10-22T09:17:08Z

x/fileurl.go

+		return nil, err
+	}
+	if u.Scheme == "file" || u.Scheme == "" {
+		u.Path = toSlash(u.Path)


Here also, on POSIX \ could be part of the file name and the conversion is not necessary at all.

Good call! I didn't think of this. I will make some tests and try to fix it

I think I have fixed this issue now

zepatrik · 2020-10-22T09:18:35Z

x/fileurl.go

+}
+
+func stripFistPathSeparators(fPath string) string {
+	for len(fPath) > 0 && (fPath[0] == '/' || fPath[0] == '\\') {


Here also, on POSIX \ could be part of the file name.

zepatrik · 2020-10-22T09:21:25Z

x/fileurl_test.go

+		{"file://file7.txt", "file7.txt", "file7.txt"},
+		{"file://path/file8.txt", "path/file8.txt", "path/file8.txt"},
+		{"file://C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\9ccf9f68-121c-451a-8a73-2aa360925b5a386398343/access-rules.json", "/C:/Users/RUNNER~1/AppData/Local/Temp/9ccf9f68-121c-451a-8a73-2aa360925b5a386398343/access-rules.json", "file:///C:/Users/RUNNER~1/AppData/Local/Temp/9ccf9f68-121c-451a-8a73-2aa360925b5a386398343/access-rules.json"},
+		{"file:///C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\9ccf9f68-121c-451a-8a73-2aa360925b5a386398343/access-rules.json", "/C:/Users/RUNNER~1/AppData/Local/Temp/9ccf9f68-121c-451a-8a73-2aa360925b5a386398343/access-rules.json", "file:///C:/Users/RUNNER~1/AppData/Local/Temp/9ccf9f68-121c-451a-8a73-2aa360925b5a386398343/access-rules.json"},


I am not quite sure this is what we want. Is this following some spec or standard?

Well, this can be discussed of course, but I have determined that this is a reasonable solution since this style of non standard file URLs are used throughout Oathkeeper. And since the file scheme doesn't support relative paths the solution is to use an empty scheme. This is also documented in the standard library documentation: https://golang.org/pkg/net/url/#Parse
I have tried to combine the standard and also the way many people are probably using file URLs already without having any regressions.

zepatrik · 2020-10-22T09:27:30Z

x/fileurl.go

+		return ""
+	}
+
+	if u.Scheme != "file" && u.Scheme != "" {


Let's invert this condition to make it more readable

Suggested change

if u.Scheme != "file" && u.Scheme != "" {

if !(u.Scheme == "file" || u.Scheme == "") {

Edit: I forgot the not 🤦‍♂️

Hi Mr. De Morgan 👋

This is fixed

strings.Index() == 0 -> strings.HasPrefix() Co-authored-by: Patrik <[email protected]>

Namingconvention Co-authored-by: Patrik <[email protected]>

Also fixed a few issues with \ on POSIX where it should not be interpreted as a path separator.

PatrLind · 2020-10-22T12:10:54Z

So I have made some changes after your suggestions now.
As for the runtime.GOOS == "windows", I will address this in the urlx package by adding the appropriate build tags for the different versions.

zepatrik

Nice, this looks good now 🎉
Let's merge it in ory/x

zepatrik · 2020-10-22T12:20:49Z

x/fileurl.go

+		return url.Parse(rawURL)
+	}
+
+	if strings.HasPrefix(lcRawURL, "file://") {


I just realized you can avoid the if statement and use strings.TrimPrefix(lcRawURL, "file://"), sry 😅

I has the same thought, but it turns out that we are comparing the lower case converted string but modifying the non converted string so unfortunately that won't work. Unless there is a way to TrimPrefix without case sensitivity?

Ah no, you are actually trimming rawURL, nevermind.

I added a trimPrefixIC function to improve the readability of the code. Also removed some unessesary code

zepatrik · 2020-11-16T09:31:49Z

What is the current state of this @PatrLind ? Do you think it is enough to bump the ory/x version for oathkeeper or are any additional changes needed?

The code is now in urlx, I changed the ParseOrXXX functions to use urlx.Parse() instead of url.Parse() as it still is in urlx.

PatrLind · 2020-11-17T03:07:28Z

I have been a bit busy on other things so this was kind of forgotten.
I have updated the ory/x version now. The only problem seems to be the failing Windows test:
TestFetcherWatchRepositoryFromFS/case=2
I am not sure why, is it related to this you think?

zepatrik · 2020-11-17T14:43:48Z

Hm, in essence this means that file watching does not work on windows? Although it worked for the first case. Maybe it is flaky? Can you reproduce that locally? I don't have a windows machine at hand.

zepatrik · 2020-11-17T14:58:00Z

Damn, the windows tests are flaky AF... I just reran it and rules passed now but other tests are failing now.

zepatrik · 2020-11-17T15:10:10Z

Github actions overwrites logs on rerun 😪 here are at least some logs I could gather:

Maybe you can have a look if you happen to have time? But I think this is not related to this PR anyway...

PatrLind · 2020-11-18T07:02:43Z

Ok, I have dived deep in the rabbit hole...
It turns out that the issue is caused by the use of time.Now() for telling when the config has changed in viper (https://github.com/ory/viper/blob/8cc0271ae7b63f6a696a9e586de4250dca19f33c/viper.go#L199).
This is in turn causing a problem with cache invalidation in the oathkeeper viper provider.

oathkeeper/driver/configuration/provider_viper.go

Line 277 in 45d5914

ts := viper.ConfigChangeAt().UnixNano()

The problem with using time.Now() is that the precision of the time value is different on different operating systems. On Windows (at least my machine) it seems to give increase the time value every 1 millisecond.
Also, I think there might be another issue when the system clock is adjusted (on any system) that the timestamp can go backwards or stay the same for some longer time.
Anyway this will cause issues for cached values and can potentially be a security issue as well if the configuration isn't applied as the user thinks it should be.

So to recap. The reason the tests fail on Windows is because the old configuration is cached. This happens randomly depending on the timing of the tests. It will probably fail more on a fast computer than a slow computer.

So what is the solution? I am not exactly sure, but I think a good way to start would be to not use the configChangedAt in viper for anything related to caching or to tell if the configuration has changed. It would be better to use a configuration version number that increases monotonically for each change instead.
Also the hashPipelineConfig function scares me a bit since it creates a hash value and uses that for caching configuration. Seems to me that this should be a more cryptographically safe hash function instead.

oathkeeper/driver/configuration/provider_viper.go

Line 293 in 45d5914

return crc64.Checksum(hashSlices, crc64.MakeTable(crc64.ECMA)), nil

Edit:
Also, I think this is a potential data race:

oathkeeper/driver/configuration/provider_viper.go

Line 273 in 45d5914

return v.enabledCache[hash]

aeneasr · 2020-11-18T09:36:35Z

@PatrLind I think you have found ory/x#216

zepatrik · 2020-11-18T09:40:26Z

Thanks for your debugging effort 👍
Should we merge this PR as is? The failing cases in the CI definitely seem to be viper related.

PatrLind · 2020-11-18T09:43:14Z

I guess you can merge, but no rush if you want to do more validation.

See ory/oathkeeper#557 (comment) See ory/x#216

aeneasr · 2020-11-18T10:20:57Z

Hm so I looked into this and while your observations will certainly be correct, ConfigChangedAt is actually not used anywhere across ory/x, ory/hydra, ory/oathkeeper nor ory/viper itself. So I don't think that that could be the root cause of these problems.

However, I think this shouldn't be part of this PR anyways. Thank you so, so much for your hard work! @vinckr will send some goodies your way :)

PatrLind · 2020-11-18T10:40:49Z

@aeneasr oh, thanks! :)
But I was a bit confused by the statement that ConfigChangedAt isn't used. Indeed if I do a search in oathkeeper there is no match. But if you instead do a search for ConfigChangeAt (the name of the function that returns the time value), you will get some matches. But perhaps that code in itself isn't used anywhere?

aeneasr · 2020-11-18T10:43:50Z

Ah, my bad - I still think that this is not what is causing the issue (even though it definitely is an issue!) as the problem with the configs not being properly read in also appears in hydra, which does not use this method or value. In any case, I will try to debug this in ORY Hydra now and then will probably come back to the windows tests here!

PatrLind added 5 commits October 15, 2020 15:08

fix: file urls doesn't work in Windows

59f3d80

Fixes so that file urls work for jwks_url

04ad034

Tests didn't run correctly when using an incorrect int to string conv…

7635fbd

…ersion

Added non standard windows path case to make tests not fail

ab0fadf

Makes the test not fail on Windows

cdcba08

zepatrik requested changes Oct 15, 2020

View reviewed changes

helper/fileurl_test.go Outdated Show resolved Hide resolved

helper/fileurl.go Outdated Show resolved Hide resolved

PatrLind added 2 commits October 19, 2020 18:43

Move from package helper to x

044eaa2

Removes support for hostname and adds special handling of all non sta…

c4d5b8c

…ndard variants of file URLs

PatrLind requested a review from zepatrik October 20, 2020 08:17

zepatrik and others added 13 commits October 20, 2020 10:29

ci: add windows unit tests

98c91ca

ci: enable windows unit tests

4262cd0

ci: run windows test as github workflow instead of CCI

f43a989

ci: escape * in yaml

8c1d4ab

Merge pull request #1 from ory/windows-ci-test

ec676bf

ci: windows unit test

New ParseURL function and fixes to GetURLFilePath

1360892

The PatseURL function will handle parsing URLs with special handling of file:// URLs The updated GetURLFilePath function will return a correct path baed on the updated URL parsing.

Fox compile error, TestGetURLFilePath expects ptr

6dbdf82

Add ParseOrPanic from xurl

91352a2

Only handle UNC paths on Windows

933e376

interpret empty scheme as file

0d489b3

Use the fixed URL parse function

0c5070c

Use the fixed URL parse helpers

2936897

Fixes handling of files with ./ path

9a24f01

zepatrik requested changes Oct 21, 2020

View reviewed changes

zepatrik requested changes Oct 22, 2020

View reviewed changes

PatrLind mentioned this pull request Oct 22, 2020

fix: Functions to parse URLs and handle URL file paths ory/x#219

Merged

5 tasks

PatrLind and others added 4 commits October 22, 2020 17:42

Update x/fileurl.go

a94d262

strings.Index() == 0 -> strings.HasPrefix() Co-authored-by: Patrik <[email protected]>

Update x/fileurl.go

940878e

Namingconvention Co-authored-by: Patrik <[email protected]>

Fixes after suggestions from zepatrik

9c61d60

Also fixed a few issues with \ on POSIX where it should not be interpreted as a path separator.

Small cleanup

fa70665

zepatrik reviewed Oct 22, 2020

View reviewed changes

PatrLind added 2 commits October 22, 2020 20:26

Removed unused function stripFistPathSeparators

69aa394

Improved readability

7a70e0c

I added a trimPrefixIC function to improve the readability of the code. Also removed some unessesary code

PatrLind added 4 commits November 17, 2020 10:26

Merge remote-tracking branch 'remotes/ory/master' into master

9ec4ad2

Updated to github.com/ory/x v0.0.163

5a96ab4

Switched to github.com/ory/x/urlx

a451fc7

The code is now in urlx, I changed the ParseOrXXX functions to use urlx.Parse() instead of url.Parse() as it still is in urlx.

go mod tidy

ba2970f

PatrLind mentioned this pull request Nov 18, 2020

I found some data race warnings #574

Open

aeneasr added a commit to ory/viper that referenced this pull request Nov 18, 2020

fix: use monotonic counter for tracking config changes

d8c0100

See ory/oathkeeper#557 (comment) See ory/x#216

aeneasr mentioned this pull request Nov 18, 2020

fix: use monotonic counter for tracking config changes ory/viper#20

Closed

aeneasr merged commit 6a05682 into ory:master Nov 18, 2020

	lcRawurl := strings.ToLower(rawurl)
	lcRawURL := strings.ToLower(rawurl)

-	lcRawurl := strings.ToLower(rawurl)
+        if runtime.GOOS == "windows" {
+	    rawURL = strings.ToLower(rawurl)
+	}

	if u.Scheme != "file" && u.Scheme != "" {
	if !(u.Scheme == "file" \|\| u.Scheme == "") {

fix: Read windows path error - 2 #557

fix: Read windows path error - 2 #557

Conversation

PatrLind commented Oct 15, 2020

Related PR: #514

Proposed changes

Checklist

Further comments

CLAassistant commented Oct 15, 2020 • edited Loading

zepatrik commented Oct 15, 2020

zepatrik left a comment

Choose a reason for hiding this comment

PatrLind commented Oct 15, 2020

PatrLind commented Oct 19, 2020

zepatrik commented Oct 19, 2020

PatrLind commented Oct 19, 2020 • edited Loading

PatrLind commented Oct 20, 2020 • edited Loading

zepatrik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zepatrik Oct 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PatrLind commented Oct 22, 2020

zepatrik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PatrLind Oct 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zepatrik commented Nov 16, 2020

PatrLind commented Nov 17, 2020

zepatrik commented Nov 17, 2020

zepatrik commented Nov 17, 2020

zepatrik commented Nov 17, 2020

PatrLind commented Nov 18, 2020 • edited Loading

aeneasr commented Nov 18, 2020

zepatrik commented Nov 18, 2020

PatrLind commented Nov 18, 2020

aeneasr commented Nov 18, 2020

PatrLind commented Nov 18, 2020

aeneasr commented Nov 18, 2020

CLAassistant commented Oct 15, 2020 •

edited

Loading

PatrLind commented Oct 19, 2020 •

edited

Loading

PatrLind commented Oct 20, 2020 •

edited

Loading

zepatrik Oct 22, 2020 •

edited

Loading

PatrLind Oct 22, 2020 •

edited

Loading

PatrLind commented Nov 18, 2020 •

edited

Loading