Skip to content
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

[Adding] support for account_token_position #1874

Merged
merged 2 commits into from Feb 2, 2021

Conversation

matthiashanel
Copy link
Contributor

@matthiashanel matthiashanel commented Feb 1, 2021

Signed-off-by: Matthias Hanel mh@synadia.com

This was added to support ngs.

This change does 4 things.
Refactor to only have one function to validate imports.
Have This function support the jwt field account_token_position.
For completeness make this value configurable as well.
unit tests

Signed-off-by: Matthias Hanel <mh@synadia.com>
Copy link
Member

@aricart aricart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - this will simplify the securing of imports without requiring a token.

Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments. If the new API does not need to be public, my comment on using private name applies everywhere where the new API is referenced. I did not comment in all occurrences.

server/accounts.go Outdated Show resolved Hide resolved
server/accounts.go Show resolved Hide resolved
server/accounts.go Outdated Show resolved Hide resolved
server/accounts.go Outdated Show resolved Hide resolved
server/accounts.go Outdated Show resolved Hide resolved
server/accounts.go Outdated Show resolved Hide resolved
server/accounts.go Outdated Show resolved Hide resolved
server/accounts.go Outdated Show resolved Hide resolved
server/accounts.go Outdated Show resolved Hide resolved
Signed-off-by: Matthias Hanel <mh@synadia.com>
@@ -3,9 +3,11 @@ github.com/golang/protobuf v1.4.0-rc.1.0.20200221234624-67d41d38c208/go.mod h1:x
github.com/golang/protobuf v1.4.0-rc.2/go.mod h1:LlEzMj4AhA7rCAGe4KMBDvJI+AwstrUpVNzEA03Pprs=
github.com/golang/protobuf v1.4.0-rc.4.0.20200313231945-b860323f09d0/go.mod h1:WU3c8KckQ9AFe+yFwt9sWVRKCVIyN9cPHBJSNnbL67w=
github.com/golang/protobuf v1.4.0/go.mod h1:jodUvKwWbYaEsadDk5Fwe5c77LiNKVO9IDvqG2KuDX0=
github.com/golang/protobuf v1.4.2 h1:+Z5KGCizgyZCbGh1KZqA0fcLLkwbsjIzS4aV2v7wJX0=
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, any reason for changes in the go.sum? Could you make sure that you have rebased from master and that these changes are required?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe missing in an earlier commit?

> git co account_protected_export
Switched to branch 'account_protected_export'
> git rebase origin/master
Current branch account_protected_export is up to date.
> go mod vendor
> go mod tidy
> git status
On branch account_protected_export
Untracked files:
  (use "git add <file>..." to include in what will be committed)
	mytest
	server.test
nothing added to commit but untracked files present (use "git add" to track)
>

Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants