-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Start using AuthProvider audiences. #13
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
Changes from all commits
eba2bfe
ca9ef31
c2438b7
ab52aa1
d1a67c6
af9709d
c1c644d
137e25b
0c0c76f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| test --test_output=errors | ||
| test --test_size_filters=-large,-enormous |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| # This is from Bazel's former travis setup, to avoid blowing up the RAM usage. | ||
| startup --host_jvm_args=-Xmx2500m | ||
| startup --host_jvm_args=-Xms2500m | ||
| startup --batch | ||
| test --ram_utilization_factor=10 | ||
|
|
||
| # This is so we understand failures better | ||
| build --verbose_failures | ||
|
|
||
| # Below this line, .travis.yml will cat the default bazelrc. | ||
| # This is needed so Bazel starts with the base workspace in its | ||
| # package path. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| /bazel-* |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| sudo: required | ||
| dist: xenial | ||
|
|
||
| lang: go | ||
|
|
||
| go: | ||
| - 1.7.x | ||
|
|
||
| jdk: | ||
| - oraclejdk8 | ||
|
|
||
| env: | ||
| - BAZEL_VERSION=0.4.2 | ||
|
|
||
| addons: | ||
| apt: | ||
| packages: | ||
| - wget | ||
|
|
||
| cache: | ||
| directories: | ||
| - $HOME/bazel/install | ||
| - $HOME/bazel/outbase | ||
|
|
||
| before_install: | ||
| - mkdir -p ${HOME}/bazel/install | ||
| - cd ${HOME}/bazel/install | ||
| - wget --no-clobber "https://github.com/bazelbuild/bazel/releases/download/${BAZEL_VERSION}/bazel_${BAZEL_VERSION}-linux-x86_64.deb" | ||
| - chmod +x bazel_${BAZEL_VERSION}-linux-x86_64.deb | ||
| - sudo dpkg -i bazel_${BAZEL_VERSION}-linux-x86_64.deb | ||
| - sudo apt-get -f install -qqy uuid-dev | ||
| - cd ${TRAVIS_BUILD_DIR} | ||
| - mv .bazelrc .bazelrc.orig | ||
| - cat .bazelrc.travis .bazelrc.orig > .bazelrc | ||
|
|
||
| script: | ||
| - bazel --output_base=${HOME}/bazel/outbase test //... | ||
|
|
||
| notifications: | ||
| slack: istio-dev:wEEEbaabdP5ieCgDOFetA9nX |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -258,7 +258,7 @@ bool Config::LoadRpcMethods(ApiManagerEnvInterface *env, | |
| bool Config::LoadAuthentication(ApiManagerEnvInterface *env) { | ||
| // Parsing auth config. | ||
| const ::google::api::Authentication &auth = service_.authentication(); | ||
| map<string, string> provider_id_issuer_map; | ||
| map<string, const ::google::api::AuthProvider*> provider_id_provider_map; | ||
| for (const auto &provider : auth.providers()) { | ||
| if (provider.id().empty()) { | ||
| env->LogError("Missing id field in AuthProvider."); | ||
|
|
@@ -274,7 +274,7 @@ bool Config::LoadAuthentication(ApiManagerEnvInterface *env) { | |
| } else { | ||
| SetJwksUri(provider.issuer(), string(), true); | ||
| } | ||
| provider_id_issuer_map[provider.id()] = provider.issuer(); | ||
| provider_id_provider_map[provider.id()] = &provider; | ||
| } | ||
|
|
||
| for (const auto &rule : auth.rules()) { | ||
|
|
@@ -296,12 +296,16 @@ bool Config::LoadAuthentication(ApiManagerEnvInterface *env) { | |
| env->LogError(error.c_str()); | ||
| continue; | ||
| } | ||
| auto issuer = utils::FindOrNull(provider_id_issuer_map, provider_id); | ||
| if (issuer == nullptr) { | ||
| auto provider = utils::FindPtrOrNull(provider_id_provider_map, | ||
| provider_id); | ||
| if (provider == nullptr) { | ||
| std::string error = "Undefined provider_id: " + provider_id; | ||
| env->LogError(error.c_str()); | ||
| } else { | ||
| (*method)->addAudiencesForIssuer(*issuer, requirement.audiences()); | ||
| const std::string &audiences = provider->audiences().empty() | ||
| ? requirement.audiences() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. const string&
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
| : provider->audiences(); | ||
| (*method)->addAudiencesForIssuer(provider->issuer(), audiences); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -304,6 +304,7 @@ static const char auth_config[] = | |
| " id: \"provider-id1\"\n" | ||
| " issuer: \"issuer1@gserviceaccount.com\"\n" | ||
| " jwks_uri: \"https://www.googleapis.com/jwks_uri1\"\n" | ||
| " audiences: \"ok_audience1\"\n" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we add a test case with provider without audience bur requirement has.?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is already there. There are two apis with AuthRequirements and both have audiences right now. I have just moved one out to AuthProvider and the other one has AuthRequirements audience set. |
||
| " }\n" | ||
| " providers {\n" | ||
| " id: \"provider-id2\"\n" | ||
|
|
@@ -326,7 +327,6 @@ static const char auth_config[] = | |
| " selector: \"Xyz.Method1\"\n" | ||
| " requirements {\n" | ||
| " provider_id: \"provider-id1\"\n" | ||
| " audiences: \"ok_audience1\"\n" | ||
| " }\n" | ||
| " }\n" | ||
| " rules {\n" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to make a copy, just use const provider*