Gitlab project #630

athoune · 2020-06-24T07:30:18Z

Gitlab project

--github-repo is already a setting, here is the --gitlab-project setting.

Motivation and Context

Filtering by group is an oidc option, but filtering by project is easier.

How Has This Been Tested?

There is unit test.

Functional test are done with this command :

./oauth2-proxy \
    --upstream=http://localhost:5000 \
    --oidc-issuer-url=https://gitlab.example.com \
    --cookie-secret=oKailu9ees5daiW6To0aangie0ootaif \
    --client-id=blabla_id \
    --client-secret=blabla_secret \
    --gitlab-project=group/project \
    --provider gitlab \
    --scope "openid email read_api" \
    --cookie-domain project.example.com \
    --standard-logging true\
    --email-domain=example.com

Checklist:

My change requires a change to the documentation or CHANGELOG.
I have updated the documentation/CHANGELOG accordingly.
I have created a feature (non-master) branch for my PR.

athoune · 2020-06-25T10:03:05Z

The codeclimate advice is weird.

NickMeves · 2020-06-27T03:12:49Z

providers/gitlab_test.go

@@ -108,6 +127,46 @@ func TestGitLabProviderUsername(t *testing.T) {
 	assert.Equal(t, "FooBar", username)
 }

+func TestGitlabProviderProject(t *testing.T) {


I'm taking a look at these units tests for the first time myself since I haven't used Gitlab -- I see alot of redundant test code (historically before you).

Do you have the bandwidth to try to to get these setup using table driven tests?

@NickMeves Hi, is something like this https://github.com/factorysh/oauth2-proxy/commit/f2480880919359a32d6a9137ff7987e868e1cf92, the way you want it ? If so, i'll finish and clean up. Thanks for your reply. (i searched briefly in the repo for reference and found nothing using table tests)

Everything is updated using table tests.

This has some samples: https://github.com/oauth2-proxy/oauth2-proxy/blob/master/pkg/apis/sessions/session_state_test.go

Key point is using the t.Run method to take the test case name and make a subtest out of it.

We are also trying wherever possible to migrate new tests to using ginkgo more (you can see an example of adding that to legacy test tables here:

oauth2-proxy/pkg/apis/sessions/session_state_test.go

Line 21 in d7abd56

g := NewWithT(t)

)

If you went full Ginkgo BDD, here is a sample of a Test Table with that framework we are using for alot of new tests: https://github.com/oauth2-proxy/oauth2-proxy/blob/master/pkg/requests/result_test.go

Thanks for reply, never heard of this framework but it was quite fun to use, full migration done in fecd956

NickMeves · 2020-06-27T03:13:26Z

providers/gitlab_test.go

+	b := testGitLabBackend()
+	defer b.Close()
+
+	bURL, _ := url.Parse(b.URL)


I see the older tests do this too -- but lets not ignore the errors.

Can we add a assert.NoError(t, err) after this?

NickMeves · 2020-06-27T03:13:54Z

providers/gitlab_test.go

+	p.Project = "my_group/my_bad_project"
+	session := &sessions.SessionState{AccessToken: "gitlab_access_token"}
+	_, err := p.GetEmailAddress(context.Background(), session)
+	assert.NotNil(t, err)


Suggested change

assert.NotNil(t, err)

assert.Error(t, err)

NickMeves · 2020-06-27T03:14:44Z

providers/gitlab_test.go

+	p.Project = "my_group/my_project"
+	session := &sessions.SessionState{AccessToken: "gitlab_access_token"}
+	email, err := p.GetEmailAddress(context.Background(), session)
+	assert.Equal(t, nil, err)


Suggested change

assert.Equal(t, nil, err)

assert.NoError(t, err)

NickMeves · 2020-06-27T03:19:02Z

providers/gitlab.go

+	}
+	var body []byte
+	body, err = ioutil.ReadAll(resp.Body)
+	resp.Body.Close()


I'm on a warpath for unhandled closes today to eventually get to a clean Gosec 😄 -- can we handle the error response from this?

NickMeves · 2020-06-27T03:21:52Z

providers/gitlab.go

@@ -250,6 +295,17 @@ func (p *GitLabProvider) GetEmailAddress(ctx context.Context, s *sessions.Sessio
 		return "", fmt.Errorf("group membership check failed: %v", err)
 	}

+	// Check project membership


Can we make this a verifyProjectMembership function to mirror the organization of the other verifiers that are similar to this above?

NickMeves · 2020-06-27T03:25:58Z

docs/configuration/configuration.md

@@ -58,6 +58,7 @@ An example [oauth2-proxy.cfg]({{ site.gitweb }}/contrib/oauth2-proxy.cfg.example
 | `--github-token` | string | the token to use when verifying repository collaborators (must have push access to the repository) | |
 | `--github-user` | string \| list | To allow users to login by username even if they do not belong to the specified org and team or collaborators | |
 | `--gitlab-group` | string | restrict logins to members of any of these groups (slug), separated by a comma | |
+| `--gitlab-project` | string | restrict logins to members of any of these project | |


This just merged and fixes an issue with the --gitlab-group config setup (singular vs plural and StringSlice flag): #639

I think you likely want to mirror those changes for projects assuming you also want to allow multiples.

I made the changes for multiple groups, not sure about how you want to log failed attempts. Can you give me an advice for this ? Thanks a lot.

NickMeves

Thanks for the PR! I'm not fully familiar with the GitLab provider myself, so take my suggestions with a grain of salt if they seem completely out of whack 😄

JoelSpeed

Big fan of the ginkgo rewrite of the gitlab provider tests, thanks for that!

JoelSpeed · 2020-08-28T14:53:51Z

pkg/apis/options/options.go

@@ -51,6 +51,7 @@ type Options struct {
 	GitHubToken              string   `flag:"github-token" cfg:"github_token"`
 	GitHubUsers              []string `flag:"github-user" cfg:"github_users"`
 	GitLabGroup              string   `flag:"gitlab-group" cfg:"gitlab_group"`
+	GitlabProject            []string `flag:"gitlab-project" cfg:"gitlab_projects"`


Shouldn't this be named GitlabProjects since it's a list?

For me yes but I found this in upstream :

GitLabGroup []string `flag:"gitlab-group" cfg:"gitlab_groups" in https://github.com/oauth2-proxy/oauth2-proxy/blob/master/pkg/apis/options/options.go#L51

And done it the same way

I'd rather it was with the trailing s, this is what the other should be as well IMO

JoelSpeed · 2020-08-28T14:54:13Z

pkg/apis/options/options.go

@@ -232,6 +233,7 @@ func NewFlagSet() *pflag.FlagSet {
 	flagSet.String("github-token", "", "the token to use when verifying repository collaborators (must have push access to the repository)")
 	flagSet.StringSlice("github-user", []string{}, "allow users with these usernames to login even if they do not belong to the specified org and team or collaborators (may be given multiple times)")
 	flagSet.String("gitlab-group", "", "restrict logins to members of this group")
+	flagSet.StringSlice("gitlab-project", []string{}, "restrict logins to members of this project (may be given multiple times)")


Should include the formatting instructions in this

JoelSpeed · 2020-08-28T14:55:42Z

providers/gitlab.go

@@ -32,7 +35,7 @@ func NewGitLabProvider(p *ProviderData) *GitLabProvider {
 	p.ProviderName = "GitLab"

 	if p.Scope == "" {
-		p.Scope = "openid email"
+		p.Scope = "openid email read_api"


Perhaps only make this default change if projects are specified? Otherwise we may break existing users applications

What's the best approach for this ? I can't setup a condition inside New, because there is no info about projects at this time in the execution. In the github provider, there is method to set group https://github.com/oauth2-proxy/oauth2-proxy/blob/master/providers/github.go#L95, I can duplicate this behavior in the gitlab provider and ensure in this method that "read_api" will be added to scope only once. Thanks for reply.

JoelSpeed · 2020-08-28T15:00:46Z

providers/gitlab.go

+		Path:   "/api/v4/projects/",
+	}
+	endpoint := endpointURL.String() + url.QueryEscape(project)
+	req, err := http.NewRequestWithContext(ctx, "GET", endpoint, nil)


Please use the request builder from github.com/oauth2-proxy/oauth2-proxy/pkg/requests as can be seen in other providers

oauth2-proxy/providers/github.go

Lines 131 to 138 in 37026b6

err := requests.New(endpoint.String()).

WithContext(ctx).

WithHeaders(makeGitHubHeader(accessToken)).

Do().

UnmarshalInto(&op)

if err != nil {

return false, err

}

I think this should work

err := requests.New(endpoint). WithContext(ctx). SetHeader("Authorization", "Bearer "+s.AccessToken). Do(). UnmarshalInto(&projectInfo) if err != nil { return false, err }

JoelSpeed · 2020-08-28T15:03:27Z

providers/gitlab.go

+
+	for _, project := range p.Projects {
+		project, err := p.getProjectInfo(ctx, s, project)
+		if err == nil && !project.Archived {


I'm assuming no error means that the user is a member of the project? This could probably do with a comment to explain if that's the case

JoelSpeed · 2020-08-28T15:05:19Z

providers/gitlab_test.go

 	authHeader := "Bearer gitlab_access_token"

 	return httptest.NewServer(http.HandlerFunc(
 		func(w http.ResponseWriter, r *http.Request) {
-			if r.URL.Path == "/oauth/userinfo" {
+			if r.URL.String() == "/oauth/userinfo" {


Not sure I understand why this change was made? Can you remember?

I wonder if it's worth making this a switch statement rather than a series of if statements 🤔

Not sure about why my team mate did this thing 🤷. Ok, it was about URL encoding and how to interpret /, I just fixed it.

JoelSpeed · 2020-08-28T15:06:56Z

providers/gitlab_test.go

-	assert.Equal(t, nil, err)
-	assert.Equal(t, "foo@bar.com", email)
-}
+var _ = Describe("Gitlab Provider Tests", func() {


Nice rewrite of the tests, thanks!!! 😄

JoelSpeed · 2020-08-28T15:08:55Z

providers/gitlab_test.go

+				if in.projects != nil {
+					if len(in.projects) >= 1 {
+						p.Projects = in.projects
+					}
+				}


I think this can be simplified

Suggested change

if in.projects != nil {

if len(in.projects) >= 1 {

p.Projects = in.projects

}

}

if len(in.projects) > 0 {

p.Projects = in.projects

}

Oh yes, that's dumb, I forgot that a nil slice have a length of 0. 😹

JoelSpeed · 2020-08-28T16:49:38Z

There are a lot of conflicts on this branch, can you rebase it and resolve the conflicts?

papey · 2020-08-28T17:24:43Z

Sure, i'll clean up the mess to give you a clean PR, thanks for all the feedback.

papey · 2020-09-01T15:32:17Z

This branch is now rebased against master, there is still some real world tests to do to ensure we do not hit some corner cases.

In this implementation, a user can not access an archived repo, this behavior can be discuss if needed since archived can not mean the same thing for each users.

I just need to migrate to the requests package as requested by @JoelSpeed.

Apart from that, I think all the requested changes are updated with all the advices and insights we discuss in this conversation.

papey · 2020-09-02T13:14:54Z

I've test it against our production server, users member of target project can access resources, users not member of target project get a 403.

Do you have any rules/guide to update Docs and Changelog ?

Do you want me to squash everything before merge ?

Thanks.

NickMeves

Changes look good to me!

Please add a CHANGELOG entry and we can then merge 👍

NickMeves

Sorry, just when I approved I eyeballed a change I think needs to be discussed.

With that fix + a CHANGELOG entry, this will be good to go.

NickMeves · 2020-09-19T19:19:13Z

providers/gitlab.go

+// SetProjects adds Projects to a GitLabProvider struct and extends OAuth2 needed scope
+func (p *GitLabProvider) SetProjects(projects []string) {
+	p.Projects = projects
+	p.Scope += " read_api"


I think you need to be aware of when the enduser consciously updates the scope option and not clobber it when they put in custom values.

We just had to think about something similar with the OIDC provider adding group claim AuthZ:

https://github.com/oauth2-proxy/oauth2-proxy/pull/616/files#diff-2690264192c9633892c7bda108d4a58dR156

I think similar logic might be warranted here -- only add read_api if p.Scope isn't explicitly set by the user. If I'm following this logic correctly, you are always adding this to the Gitlab scope, so should you just add read_api to gitlabDefaultScope?

Just to clarify, this is dynamically added here (if projects options are specified) https://github.com/oauth2-proxy/oauth2-proxy/pull/630/files#diff-80edce26da4509c90a73a1669a5bbdc7R180. This won't break old installation already in place without this extended scope (as requested by @JoelSpeed). If something needs to be change in this behavior, tell me and i'll do it.

I think you only add read_api in SetProjects (I agree with @JoelSpeed on his conditionality point) if the user didn't set something explicitly themselves for the scope 🤔

E.g. If I explicitly set the --scope to openid email read_api this code turns that into openid email read_api read_api. I feel like if a user explicitly changes the default scope for whatever reason (even if it results in bugs for them because they forgot read_api after wanting this feature), at that point our code can't attempt to mutate it.

Ok, I think I get it ! While trying to figure it out, i came across this : https://github.com/oauth2-proxy/oauth2-proxy/blob/master/pkg/validation/options.go#L153. This is what I get as default scope when creating the Gitlab Provider struct (without scope option).

I did not find a way to know if the user requested a specific scope or if it's the default one. Doing a diff is way to tricky to be maintainable.

The approach I take is : avoid duplicates read_api in the scope (ie: check if absent before adding it). I think it's the most viable solution. (See 3514af8)

I still have concerns this is always called even when project authorization isn't configured: https://github.com/oauth2-proxy/oauth2-proxy/pull/630/files#diff-2690264192c9633892c7bda108d4a58dR290

The link you sent above about the conditional SetProject call that has this logic was in a unit test.

As far as the read_api duplicate concern -- the new commit with your proposed solution looks good to me 👍

Ok, i just add a check before calling SetProjects c0e9d1c. If everything is fine, I can squash and rebase before you merge. Thanks a lot for your feedback !

NickMeves · 2020-09-22T20:24:09Z

pkg/validation/options.go

@@ -287,6 +287,9 @@ func parseProviderInfo(o *options.Options, msgs []string) []string {
 	case *providers.GitLabProvider:
 		p.AllowUnverifiedEmail = o.InsecureOIDCAllowUnverifiedEmail
 		p.Groups = o.GitLabGroup
+		if len(o.GitlabProjects) > 0 {


NIT: I think this logic belongs in SetProjects around the scope manipulation. Just to keep things together, it is detached from the area driving the reason for this logic when over here.

Avoids the double standard since we are perfectly fine setting the parallel functionality p.Groups to an empty array right above this.

If I clearly understand what you say, here is the fix : 542056e

Unfortunately, I think that commit brings us back to the original problem (scope is manipulated even when we don't need to).

This is what I was thinking:
if len(o.GitlabProjects) > 0 && !strings.Contains(p.Scope, "read_api")

Just to keep the logic together and only manipulate scope if needed.

NickMeves · 2020-09-23T18:17:43Z

Tangentially related to this PR, but likely too much scope creep, better in a different PR:

#767 (comment)

How do we want to support gitlab group or project AuthZ in the new X-Forwarded-Groups? Add Gitlab groups to the session.Groups field and ignore projects completely?

I'm curious your thoughts as a Gitlab provider user (I don't use this provider) -- I want to get all our Providers that have some form of Group AuthZ logic to pass the groups data to the backend via the session.Groups field moving forward.

papey · 2020-09-24T12:25:08Z

How do we want to support gitlab group or project AuthZ in the new X-Forwarded-Groups? Add Gitlab groups to the session.Groups field and ignore projects completely?

No sure about the semantic of the Groups keyword but projects can be seen as a Group of users (related to this project) too.

For now the matching project is not returned on success but it's quite easy to add this variable in the early return (https://github.com/oauth2-proxy/oauth2-proxy/pull/630/files#diff-83fab261d52ee47763d81d669a7b7b74R295).

I'm curious your thoughts as a Gitlab provider user (I don't use this provider) -- I want to get all our Providers that have some form of Group AuthZ logic to pass the groups data to the backend via the session.Groups field moving forward.

One way could be add a dedicated header for projects, since Github also have this feature. The other way could be passing the projects in the group header since a project is, at the end, a group of users.

NickMeves · 2020-09-27T22:23:44Z

FYI - I have 2 PRs very related to this: #796 & #797

Once they merge I will circle back and we can get this finalized and merged in as well. I really want to clean up the post token redeem session enrichment process (where a lot of providers have hacked in authorization logic into GetEmailAddress) and get a centralized method for authorization under a global Authorize provider method.

In my refactor in #796 I noted the area where Gitlab had hacky AuthZ in an improper spot and added a TODO note.

papey · 2020-09-29T15:41:43Z

Indeed, it looks like there is a lot of plumbing stuff to do to harmonize all the things and remove all the accumlated layers of hacks over hacks.

For now, the impact on this PR looks minimal and I hope it will not invalidate the work and efforts I put in.

NickMeves · 2020-11-12T20:17:49Z

Sorry for the delay. If you still want to get this over the finish line:

#797 merged.

The refactor to EnrichSessionState & Authorize provider interface methods are now in master. This should be completely unblocked now.

papey · 2020-11-13T08:57:57Z

Hi @NickMeves, thanks for your message and status update. I will work on this next week if I find the bandwith. 👍

NickMeves · 2020-11-13T21:38:57Z

Hi @NickMeves, thanks for your message and status update. I will work on this next week if I find the bandwith. 👍

Thanks again for taking this on!

I think the only change is to add any projects (or whatever else) into the session.Groups list in EnrichSessionState. That way it will automatically be available to be used by the default Authorize method if any users of this provider want authorization.

I think for things like this with multiple group-link entities, @JoelSpeed and I were leaning toward a prefix to deconflict between the possible options. (e.g. project:blahblah) being the string added to session.Groups.

papey · 2020-11-18T17:44:56Z

Here is a starting point using EnrichSessionState, consider this as WIP but comments and feedback are welcome.

I squashed everything in order to make things more readable. I also made a copy of the old branch, just for sanity (see gitlab_project-before-enrich).

I tested it on our test project and it seems to work (needs a recheck with a fresh brain)

Also a question/opinion :

For now, we loop over each project in the config file, impersonating the user to see if project is accessible (see gitlab.go) and adding it to the session state group. In this case : 1 project in the config file means 1 request to Gitlab.

I think I will try to go reverse and fetch all the projects of the user and then compare it to the ones listed in the config file.

What do you think ?

Thanks for reading/feedback.

Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

This commit fixes a bug caused by an override of the Verifier value from *ProviderData inside GitlabProvider struct

Co-authored-by: Nick Meves <nick.meves@greenhouse.io>

papey · 2020-12-05T13:47:49Z

Rebased against master 👍

NickMeves · 2020-12-05T18:58:03Z

It was a pleasure working with you on this. Thanks again!

* Add support for gitlab projets * Add group membership in state * Use prefixed allowed groups everywhere * Fix: remove unused function * Fix: rename func that add data to session * Simplify projects and groups session funcs * Add project access level for gitlab projects * Fix: default access level * Add per project access level * Add user email when missing access level * Fix: harmonize errors * Update docs and flags description for gitlab project * Add test with both projects and groups * Fix: log error message Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> * Fix: make doc a markdown link * Add notes about read_api scope for projects * Fix: Verifier override in Gitlab Provider This commit fixes a bug caused by an override of the Verifier value from *ProviderData inside GitlabProvider struct * Fix: ensure data in session before using it * Update providers/gitlab.go Co-authored-by: Nick Meves <nick.meves@greenhouse.io> * Rename gitlab project initializer * Improve return value readbility * Use splitN * Handle space delimiters in set project scope * Reword comment for AddProjects * Fix: typo * Rework error handling in addProjectsToSession * Reduce branching complexity in addProjectsToSession * Fix: line returns * Better comment for addProjectsToSession * Fix: enrich session comment * Fix: email domains is handled before provider mechanism * Add archived project unit test * Fix: emails handling in gitlab provider Co-authored-by: Wilfried OLLIVIER <wollivier@bearstech.com> Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> Co-authored-by: Nick Meves <nick.meves@greenhouse.io>

athoune requested review from JoelSpeed and steakunderscore as code owners June 24, 2020 07:30

NickMeves reviewed Jun 27, 2020

View reviewed changes

NickMeves requested changes Jun 27, 2020

View reviewed changes

athoune requested a review from a team as a code owner August 25, 2020 15:58

JoelSpeed reviewed Aug 28, 2020

View reviewed changes

NickMeves approved these changes Sep 19, 2020

View reviewed changes

NickMeves requested changes Sep 19, 2020

View reviewed changes

NickMeves reviewed Sep 22, 2020

View reviewed changes

NickMeves mentioned this pull request Oct 20, 2020

[QUESTION] What is the current recommended GitLab.com config #857

Closed

Wilfried OLLIVIER and others added 23 commits December 4, 2020 21:08

Fix: harmonize errors

f089fd1

Update docs and flags description for gitlab project

2b33885

Add test with both projects and groups

a48ca0b

Fix: log error message

170c210

Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

Fix: make doc a markdown link

2f4179e

Add notes about read_api scope for projects

8327a70

Fix: Verifier override in Gitlab Provider

f0af811

This commit fixes a bug caused by an override of the Verifier value from *ProviderData inside GitlabProvider struct

Fix: ensure data in session before using it

f3eba6c

Update providers/gitlab.go

db2c56e

Co-authored-by: Nick Meves <nick.meves@greenhouse.io>

Rename gitlab project initializer

93a437e

Improve return value readbility

4467c6d

Use splitN

4319e92

Handle space delimiters in set project scope

7711bc3

Reword comment for AddProjects

eb296bf

Fix: typo

535ec78

Rework error handling in addProjectsToSession

9735abd

Reduce branching complexity in addProjectsToSession

68a9778

Fix: line returns

c206e31

Better comment for addProjectsToSession

c0eeeef

Fix: enrich session comment

f940c79

Fix: email domains is handled before provider mechanism

dd63dce

Add archived project unit test

d1d98f5

Fix: emails handling in gitlab provider

c3028b0

papey dismissed NickMeves’s stale review via c3028b0 December 4, 2020 20:09

NickMeves approved these changes Dec 5, 2020

View reviewed changes

NickMeves merged commit d67d6e3 into oauth2-proxy:master Dec 5, 2020

	err := requests.New(endpoint.String()).
	WithContext(ctx).
	WithHeaders(makeGitHubHeader(accessToken)).
	Do().
	UnmarshalInto(&op)
	if err != nil {
	return false, err
	}

Gitlab project #630

Gitlab project #630

Conversation

athoune commented Jun 24, 2020 • edited Loading

Gitlab project

Motivation and Context

How Has This Been Tested?

Checklist:

athoune commented Jun 25, 2020

Choose a reason for hiding this comment

papey Aug 25, 2020 • edited Loading

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

NickMeves left a comment

Choose a reason for hiding this comment

JoelSpeed left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

papey Aug 28, 2020 • edited Loading

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

papey Aug 31, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

papey Aug 28, 2020 • edited Loading

Choose a reason for hiding this comment

JoelSpeed commented Aug 28, 2020

papey commented Aug 28, 2020

papey commented Sep 1, 2020 • edited Loading

papey commented Sep 2, 2020 • edited Loading

NickMeves left a comment

Choose a reason for hiding this comment

NickMeves left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

papey Sep 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

papey Sep 21, 2020 • edited Loading

Choose a reason for hiding this comment

NickMeves Sep 21, 2020 • edited Loading

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

NickMeves commented Sep 23, 2020

papey commented Sep 24, 2020

NickMeves commented Sep 27, 2020 • edited Loading

papey commented Sep 29, 2020

NickMeves commented Nov 12, 2020

papey commented Nov 13, 2020

NickMeves commented Nov 13, 2020

papey commented Nov 18, 2020

papey commented Dec 5, 2020

NickMeves commented Dec 5, 2020

athoune commented Jun 24, 2020 •

edited

Loading

papey Aug 25, 2020 •

edited

Loading

papey Aug 28, 2020 •

edited

Loading

papey Aug 31, 2020 •

edited

Loading

papey Aug 28, 2020 •

edited

Loading

papey commented Sep 1, 2020 •

edited

Loading

papey commented Sep 2, 2020 •

edited

Loading

papey Sep 19, 2020 •

edited

Loading

papey Sep 21, 2020 •

edited

Loading

NickMeves Sep 21, 2020 •

edited

Loading

NickMeves commented Sep 27, 2020 •

edited

Loading