From af3a7bb90e927618f84e252f27be106f86b6ca20 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Fri, 21 Jul 2017 16:49:08 -0700 Subject: [PATCH 1/4] Read authorization_url in config. --- include/api_manager/method.h | 6 +++ .../api_manager/request_handler_interface.h | 3 ++ repositories.bzl | 2 +- src/api_manager/config.cc | 3 +- src/api_manager/config_test.cc | 8 ++++ src/api_manager/context/request_context.cc | 11 ++++++ src/api_manager/context/request_context.h | 3 ++ src/api_manager/method_impl.cc | 37 +++++++++++++++---- src/api_manager/method_impl.h | 23 +++++++++--- src/api_manager/method_test.cc | 31 ++++++++++------ src/api_manager/request_handler.cc | 4 ++ src/api_manager/request_handler.h | 2 + 12 files changed, 107 insertions(+), 26 deletions(-) diff --git a/include/api_manager/method.h b/include/api_manager/method.h index 03188aee4..ebcadc234 100644 --- a/include/api_manager/method.h +++ b/include/api_manager/method.h @@ -56,6 +56,12 @@ class MethodInfo { const std::string &issuer, const std::set &jwt_audiences) const = 0; + // Return authorization url for an issuer if specified. + virtual const std::string &authorization_url_by_issuer( + const std::string &issuer) const = 0; + // Return the first authorization url for this method. + virtual const std::string &first_authorization_url() const = 0; + // Get http header system parameters by name. virtual const std::vector *http_header_parameters( const std::string &name) const = 0; diff --git a/include/api_manager/request_handler_interface.h b/include/api_manager/request_handler_interface.h index 8fbcb3e1f..1804deacd 100644 --- a/include/api_manager/request_handler_interface.h +++ b/include/api_manager/request_handler_interface.h @@ -59,6 +59,9 @@ class RequestHandlerInterface { // Get the method info. virtual const MethodCallInfo *method_call() const = 0; + + // Return the authorization url if authentication fails. + virtual std::string GetAuthorizationUrl() const = 0; }; } // namespace api_manager diff --git a/repositories.bzl b/repositories.bzl index 41a0dadb2..ed610bec5 100644 --- a/repositories.bzl +++ b/repositories.bzl @@ -578,7 +578,7 @@ cc_proto_library( native.new_git_repository( name = "googleapis_git", - commit = "cbf6dc5d95d497b621a5c0e35696355deff336af", # June 14, 2017 + commit = "75c3a512bce1ac7c2e9a1dd8b2c38ac3f1f5697c", # July 21, 2017 remote = "https://github.com/googleapis/googleapis.git", build_file_content = BUILD, ) diff --git a/src/api_manager/config.cc b/src/api_manager/config.cc index 906acf5ca..30f3d2869 100644 --- a/src/api_manager/config.cc +++ b/src/api_manager/config.cc @@ -324,7 +324,8 @@ bool Config::LoadAuthentication(ApiManagerEnvInterface *env) { const std::string &audiences = provider->audiences().empty() ? requirement.audiences() : provider->audiences(); - (*method)->addAudiencesForIssuer(provider->issuer(), audiences); + (*method)->addAuthProvider(provider->issuer(), audiences, + provider->authorization_url()); } } } diff --git a/src/api_manager/config_test.cc b/src/api_manager/config_test.cc index bb76d0555..01a420fde 100644 --- a/src/api_manager/config_test.cc +++ b/src/api_manager/config_test.cc @@ -314,6 +314,7 @@ static const char auth_config[] = " id: \"provider-id2\"\n" " issuer: \"issuer2@gserviceaccount.com\"\n" " jwks_uri: \"https://www.googleapis.com/jwks_uri2\"\n" + " authorization_url: \"https://www.googleapis.com/auth\"\n" " }\n" " providers {\n" " id: \"esp-auth0\"\n" @@ -371,6 +372,9 @@ TEST(Config, TestLoadAuthentication) { {"ok_audience1"})); ASSERT_FALSE(method1->isAudienceAllowed("issuer1@gserviceaccount.com", {"ok_audience2"})); + EXPECT_EQ(method1->authorization_url_by_issuer("issuer1@gserviceaccount.com"), + ""); + EXPECT_EQ(method1->first_authorization_url(), ""); const MethodInfo *method2 = config->GetMethodInfo("GET", "/xyz/method2/abc"); ASSERT_EQ("Xyz.Method2", method2->name()); @@ -381,6 +385,10 @@ TEST(Config, TestLoadAuthentication) { {"ok_audience1"})); ASSERT_TRUE(method2->isAudienceAllowed("issuer2@gserviceaccount.com", {"ok_audience2"})); + EXPECT_EQ(method2->authorization_url_by_issuer("issuer2@gserviceaccount.com"), + "https://www.googleapis.com/auth"); + EXPECT_EQ(method2->first_authorization_url(), + "https://www.googleapis.com/auth"); const MethodInfo *method3 = config->GetMethodInfo("GET", "/xyz/method3/abc"); ASSERT_EQ("Xyz.Method3", method3->name()); diff --git a/src/api_manager/context/request_context.cc b/src/api_manager/context/request_context.cc index 05da76b46..2a91a03a3 100644 --- a/src/api_manager/context/request_context.cc +++ b/src/api_manager/context/request_context.cc @@ -348,6 +348,17 @@ void RequestContext::StartBackendSpanAndSetTraceContext() { } } +std::string RequestContext::GetAuthorizationUrl() const { + if (method_call_.method_info == nullptr) { + return ""; + } + if (auth_issuer_.empty()) { + return method_call_.method_info->first_authorization_url(); + } else { + return method_call_.method_info->authorization_url_by_issuer(auth_issuer_); + } +} + } // namespace context } // namespace api_manager } // namespace google diff --git a/src/api_manager/context/request_context.h b/src/api_manager/context/request_context.h index 32c71fb6d..a77b443f2 100644 --- a/src/api_manager/context/request_context.h +++ b/src/api_manager/context/request_context.h @@ -129,6 +129,9 @@ class RequestContext { // Get the auth claims. const std::string &auth_claims() const { return auth_claims_; } + // Return the authorization url. + std::string GetAuthorizationUrl() const; + private: // Fill OperationInfo void FillOperationInfo(service_control::OperationInfo *info); diff --git a/src/api_manager/method_impl.cc b/src/api_manager/method_impl.cc index 604caaa4c..63f32941b 100644 --- a/src/api_manager/method_impl.cc +++ b/src/api_manager/method_impl.cc @@ -43,8 +43,9 @@ MethodInfoImpl::MethodInfoImpl(const string &name, const string &api_name, request_streaming_(false), response_streaming_(false) {} -void MethodInfoImpl::addAudiencesForIssuer(const string &issuer, - const string &audiences_list) { +void MethodInfoImpl::addAuthProvider(const std::string &issuer, + const string &audiences_list, + const std::string &authorization_url) { if (issuer.empty()) { return; } @@ -52,7 +53,7 @@ void MethodInfoImpl::addAudiencesForIssuer(const string &issuer, if (iss.empty()) { return; } - set &audiences = issuer_audiences_map_[iss]; + AuthProvider &provider = issuer_provider_map_[iss]; stringstream ss(audiences_list); string audience; // Audience list is comma-delimited. @@ -60,15 +61,16 @@ void MethodInfoImpl::addAudiencesForIssuer(const string &issuer, if (!audience.empty()) { // Only adds non-empty audience. std::string aud = utils::GetUrlContent(audience); if (!aud.empty()) { - audiences.insert(aud); + provider.audiences.insert(aud); } } } + provider.authorization_url = authorization_url; } bool MethodInfoImpl::isIssuerAllowed(const std::string &issuer) const { return !issuer.empty() && - issuer_audiences_map_.find(issuer) != issuer_audiences_map_.end(); + issuer_provider_map_.find(issuer) != issuer_provider_map_.end(); } bool MethodInfoImpl::isAudienceAllowed( @@ -76,15 +78,36 @@ bool MethodInfoImpl::isAudienceAllowed( if (issuer.empty() || jwt_audiences.empty() || !isIssuerAllowed(issuer)) { return false; } - const set &audiences = issuer_audiences_map_.at(issuer); + const AuthProvider &provider = issuer_provider_map_.at(issuer); for (const auto &it : jwt_audiences) { - if (audiences.find(it) != audiences.end()) { + if (provider.audiences.find(it) != provider.audiences.end()) { return true; } } return false; } +const std::string &MethodInfoImpl::authorization_url_by_issuer( + const std::string &issuer) const { + const auto &it = issuer_provider_map_.find(issuer); + if (it != issuer_provider_map_.end()) { + return it->second.authorization_url; + } else { + static std::string empty; + return empty; + } +} + +const std::string &MethodInfoImpl::first_authorization_url() const { + for (const auto &it : issuer_provider_map_) { + if (!it.second.authorization_url.empty()) { + return it.second.authorization_url; + } + } + static std::string empty; + return empty; +} + void MethodInfoImpl::process_system_parameters() { api_key_http_headers_ = http_header_parameters(api_key_parameter_name); api_key_url_query_parameters_ = url_query_parameters(api_key_parameter_name); diff --git a/src/api_manager/method_impl.h b/src/api_manager/method_impl.h index 794c1cb90..4e93107ea 100644 --- a/src/api_manager/method_impl.h +++ b/src/api_manager/method_impl.h @@ -45,6 +45,11 @@ class MethodInfoImpl : public MethodInfo { bool isAudienceAllowed(const std::string &issuer, const std::set &jwt_audiences) const; + const std::string &authorization_url_by_issuer( + const std::string &issuer) const; + + const std::string &first_authorization_url() const; + const std::vector *http_header_parameters( const std::string &name) const { return utils::FindOrNull(http_header_parameters_, name); @@ -80,10 +85,12 @@ class MethodInfoImpl : public MethodInfo { bool response_streaming() const { return response_streaming_; } - // Adds allowed audiences (comma delimated, no space) for the issuer. - // audiences_list can be empty. - void addAudiencesForIssuer(const std::string &issuer, - const std::string &audiences_list); + // Add an auth provider info for the issuer. + // audiences_list can be empty or comma delimited without space. + void addAuthProvider(const std::string &issuer, + const std::string &audiences_list, + const std::string &authorization_url); + void set_auth(bool v) { auth_ = v; } void set_allow_unregistered_calls(bool v) { allow_unregistered_calls_ = v; } void set_skip_service_control(bool v) { skip_service_control_ = v; } @@ -138,6 +145,10 @@ class MethodInfoImpl : public MethodInfo { void ProcessSystemQueryParameterNames(); private: + struct AuthProvider { + std::set audiences; + std::string authorization_url; + }; // Method name std::string name_; // API name @@ -151,8 +162,8 @@ class MethodInfoImpl : public MethodInfo { bool allow_unregistered_calls_; // Should the method skip service control. bool skip_service_control_; - // Issuers to allowed audiences map. - std::map> issuer_audiences_map_; + // Issuers to auth provider map. + std::map issuer_provider_map_; // system parameter map of parameter name to http_header name. std::map> http_header_parameters_; diff --git a/src/api_manager/method_test.cc b/src/api_manager/method_test.cc index 8a7628e8b..673c10e33 100644 --- a/src/api_manager/method_test.cc +++ b/src/api_manager/method_test.cc @@ -47,17 +47,17 @@ TEST(MethodInfo, Create) { TEST(MethodInfo, IssueAndAudiences) { MethodInfoImplPtr method_info(new MethodInfoImpl(kMethodName, "", "")); - method_info->addAudiencesForIssuer(kIssuer1, "aud1,aud2"); - method_info->addAudiencesForIssuer(kIssuer1, "aud3"); - method_info->addAudiencesForIssuer(kIssuer1, ","); - method_info->addAudiencesForIssuer(kIssuer1, ",aud4"); - method_info->addAudiencesForIssuer(kIssuer1, ""); - method_info->addAudiencesForIssuer(kIssuer1, ",aud5,,,"); - method_info->addAudiencesForIssuer(kIssuer2https, ",,,aud6"); - method_info->addAudiencesForIssuer(kIssuer3http, ""); - method_info->addAudiencesForIssuer(kIssuer3http, "https://aud7"); - method_info->addAudiencesForIssuer(kIssuer3http, "http://aud8"); - method_info->addAudiencesForIssuer(kIssuer3http, "https://aud9/"); + method_info->addAuthProvider(kIssuer1, "aud1,aud2", ""); + method_info->addAuthProvider(kIssuer1, "aud3", ""); + method_info->addAuthProvider(kIssuer1, ",", ""); + method_info->addAuthProvider(kIssuer1, ",aud4", ""); + method_info->addAuthProvider(kIssuer1, "", ""); + method_info->addAuthProvider(kIssuer1, ",aud5,,,", ""); + method_info->addAuthProvider(kIssuer2https, ",,,aud6", ""); + method_info->addAuthProvider(kIssuer3http, "", ""); + method_info->addAuthProvider(kIssuer3http, "https://aud7", ""); + method_info->addAuthProvider(kIssuer3http, "http://aud8", ""); + method_info->addAuthProvider(kIssuer3http, "https://aud9/", ""); ASSERT_TRUE(method_info->isIssuerAllowed(kIssuer1)); ASSERT_TRUE(method_info->isIssuerAllowed(kIssuer2)); @@ -88,6 +88,15 @@ TEST(MethodInfo, IssueAndAudiences) { ASSERT_FALSE(method_info->isAudienceAllowed(kIssuer1, {})); } +TEST(MethodInfo, IssueAndAuthorizationUrl) { + MethodInfoImplPtr method_info(new MethodInfoImpl(kMethodName, "", "")); + method_info->addAuthProvider(kIssuer1, "", "url1"); + method_info->addAuthProvider(kIssuer2, "aud1,aud2", ""); + EXPECT_EQ(method_info->authorization_url_by_issuer(kIssuer1), "url1"); + EXPECT_EQ(method_info->authorization_url_by_issuer(kIssuer2), ""); + EXPECT_EQ(method_info->first_authorization_url(), "url1"); +} + TEST(MethodInfo, TestParameters) { MethodInfoImplPtr method_info(new MethodInfoImpl(kMethodName, "", "")); diff --git a/src/api_manager/request_handler.cc b/src/api_manager/request_handler.cc index 90c76032a..cb2a3b6ca 100644 --- a/src/api_manager/request_handler.cc +++ b/src/api_manager/request_handler.cc @@ -144,5 +144,9 @@ std::string RequestHandler::GetRpcMethodFullName() const { } } +std::string RequestHandler::GetAuthorizationUrl() const { + return context_->GetAuthorizationUrl(); +} + } // namespace api_manager } // namespace google diff --git a/src/api_manager/request_handler.h b/src/api_manager/request_handler.h index 16407fab3..09a9cd266 100644 --- a/src/api_manager/request_handler.h +++ b/src/api_manager/request_handler.h @@ -54,6 +54,8 @@ class RequestHandler : public RequestHandlerInterface { // Get the method info. const MethodCallInfo *method_call() const { return context_->method_call(); } + virtual std::string GetAuthorizationUrl() const; + private: // The context object needs to pass to the continuation function the check // handler as a lambda capture so it can be passed to the next check handler. From a43073185b3cc83a644eaa2d378c13bc8e92dead Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Fri, 21 Jul 2017 18:44:25 -0700 Subject: [PATCH 2/4] Hookup 302 redirect for authorization url --- src/nginx/error.cc | 91 ++++++++++++++++------ src/nginx/t/auth_redirect.t | 150 ++++++++++++++++++++++++++++++++++++ 2 files changed, 218 insertions(+), 23 deletions(-) create mode 100644 src/nginx/t/auth_redirect.t diff --git a/src/nginx/error.cc b/src/nginx/error.cc index 63f2ca306..b1776ac80 100644 --- a/src/nginx/error.cc +++ b/src/nginx/error.cc @@ -33,6 +33,7 @@ #include "src/nginx/module.h" #include "src/nginx/util.h" +using ::google::protobuf::util::error::Code; using ::google::api_manager::utils::Status; namespace google { @@ -45,6 +46,8 @@ ngx_str_t application_grpc = ngx_string("application/grpc"); ngx_str_t www_authenticate = ngx_string("WWW-Authenticate"); const u_char www_authenticate_lowcase[] = "www-authenticate"; +ngx_str_t kLocation = ngx_string("Location"); +const u_char kLocationLowcase[] = "location"; ngx_str_t missing_credential = ngx_string("Bearer"); ngx_str_t invalid_token = ngx_string("Bearer, error=\"invalid_token\""); @@ -71,6 +74,66 @@ bool ngx_esp_is_error_response(ngx_http_request_t *r, ctx->http_subrequest == nullptr; } +// Returns WWW-Authenticate header for authentication/authorization +// responses. +// See https://tools.ietf.org/html/rfc6750#section-3. +ngx_int_t ngx_esp_handle_www_authenticate(ngx_http_request_t *r, + ngx_esp_request_ctx_t *ctx) { + if (r->err_status == NGX_HTTP_UNAUTHORIZED || + r->err_status == NGX_HTTP_FORBIDDEN) { + r->headers_out.www_authenticate = reinterpret_cast( + ngx_list_push(&r->headers_out.headers)); + if (r->headers_out.www_authenticate == nullptr) { + return NGX_ERROR; + } + + r->headers_out.www_authenticate->key = www_authenticate; + r->headers_out.www_authenticate->lowcase_key = + const_cast(www_authenticate_lowcase); + r->headers_out.www_authenticate->hash = + ngx_hash_key(const_cast(www_authenticate_lowcase), + sizeof(www_authenticate_lowcase) - 1); + + if (ctx->auth_token.len == 0) { + r->headers_out.www_authenticate->value = missing_credential; + } else { + r->headers_out.www_authenticate->value = invalid_token; + } + } + return NGX_OK; +} + +// If authentication fails, and authorization url is not empty, +// Reply 302 and authorization url. +ngx_int_t ngx_esp_handle_authorization_url(ngx_http_request_t *r, + ngx_esp_request_ctx_t *ctx) { + if (ctx && ctx->status.code() == Code::UNAUTHENTICATED && + ctx->status.error_cause() == utils::Status::AUTH) { + std::string url = ctx->request_handler->GetAuthorizationUrl(); + if (!url.empty()) { + r->headers_out.status = NGX_HTTP_MOVED_TEMPORARILY; + + ngx_table_elt_t *loc; + loc = reinterpret_cast( + ngx_list_push(&r->headers_out.headers)); + if (loc == nullptr) { + return NGX_ERROR; + } + + loc->key = kLocation; + loc->lowcase_key = const_cast(kLocationLowcase); + loc->hash = ngx_hash_key(const_cast(kLocationLowcase), + sizeof(kLocationLowcase) - 1); + + ngx_str_copy_from_std(r->pool, url, &loc->value); + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ESP authorization_url: %V", &loc->value); + r->headers_out.location = loc; + } + } + return NGX_OK; +} + ngx_int_t ngx_esp_error_header_filter(ngx_http_request_t *r) { ngx_esp_request_ctx_t *ctx = reinterpret_cast( ngx_http_get_module_ctx(r, ngx_esp_module)); @@ -96,30 +159,12 @@ ngx_int_t ngx_esp_error_header_filter(ngx_http_request_t *r) { r->headers_out.content_type_len = application_json.len; r->headers_out.content_type_lowcase = nullptr; - // Returns WWW-Authenticate header for authentication/authorization - // responses. - // See https://tools.ietf.org/html/rfc6750#section-3. - if (r->err_status == NGX_HTTP_UNAUTHORIZED || - r->err_status == NGX_HTTP_FORBIDDEN) { - r->headers_out.www_authenticate = reinterpret_cast( - ngx_list_push(&r->headers_out.headers)); - if (r->headers_out.www_authenticate == nullptr) { - return NGX_ERROR; - } + ngx_int_t ret; + ret = ngx_esp_handle_www_authenticate(r, ctx); + if (ret != NGX_OK) return ret; - r->headers_out.www_authenticate->key = www_authenticate; - r->headers_out.www_authenticate->lowcase_key = - const_cast(www_authenticate_lowcase); - r->headers_out.www_authenticate->hash = - ngx_hash_key(const_cast(www_authenticate_lowcase), - sizeof(www_authenticate_lowcase) - 1); - - if (ctx->auth_token.len == 0) { - r->headers_out.www_authenticate->value = missing_credential; - } else { - r->headers_out.www_authenticate->value = invalid_token; - } - } + ret = ngx_esp_handle_authorization_url(r, ctx); + if (ret != NGX_OK) return ret; } // Clear headers (refilled by subsequent NGX header filters) diff --git a/src/nginx/t/auth_redirect.t b/src/nginx/t/auth_redirect.t new file mode 100644 index 000000000..04aba7229 --- /dev/null +++ b/src/nginx/t/auth_redirect.t @@ -0,0 +1,150 @@ +# Copyright (C) Extensible Service Proxy Authors +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# 1. Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS +# OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) +# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY +# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF +# SUCH DAMAGE. +# +################################################################################ +# +use strict; +use warnings; + +################################################################################ + +use src::nginx::t::ApiManager; # Must be first (sets up import path to the Nginx test module) +use src::nginx::t::HttpServer; +use src::nginx::t::ServiceControl; +use Test::Nginx; # Imports Nginx's test module +use Test::More; # And the test framework + +################################################################################ + +# Port allocations + +my $NginxPort = ApiManager::pick_port(); +my $BackendPort = ApiManager::pick_port(); +my $ServiceControlPort = ApiManager::pick_port(); +my $RedirectPort = ApiManager::pick_port(); + +my $t = Test::Nginx->new()->has(qw/http proxy/)->plan(11); + +my $config = ApiManager::get_bookstore_service_config .<<"EOF"; +producer_project_id: "endpoints-test" +authentication { + providers { + id: "test_auth" + issuer: "628645741881-noabiu23f5a8m8ovd8ucv698lj78vv0l\@developer.gserviceaccount.com" + jwks_uri: "http://127.0.0.1/pubkey" + authorization_url: "http://127.0.0.1:${RedirectPort}/redirect" + } + rules { + selector: "ListShelves" + requirements { + provider_id: "test_auth" + audiences: "ok_audience_1" + } + } +} +control { + environment: "http://127.0.0.1:${ServiceControlPort}" +} +EOF +$t->write_file('service.pb.txt', $config); + +$t->write_file_expand('nginx.conf', <<"EOF"); +%%TEST_GLOBALS%% +daemon off; +events { + worker_connections 32; +} +http { + %%TEST_GLOBALS_HTTP%% + server_tokens off; + server { + listen 127.0.0.1:${NginxPort}; + server_name localhost; + location / { + endpoints { + api service.pb.txt; + on; + } + proxy_pass http://127.0.0.1:${BackendPort}; + } + } +} +EOF + +$t->run_daemon(\&bookstore, $t, $BackendPort, 'bookstore.log'); +$t->run_daemon(\&servicecontrol, $t, $ServiceControlPort, 'servicecontrol.log', $report_done); + +is($t->waitforsocket("127.0.0.1:${BackendPort}"), 1, 'Bookstore socket ready.'); +is($t->waitforsocket("127.0.0.1:${ServiceControlPort}"), 1, 'Service control socket ready.'); + +$t->run(); + +################################################################################ +my $response = ApiManager::http_get($NginxPort, "/shelves"); + +$t->stop_daemons(); + +like($response, qr/HTTP\/1\.1 401 Unauthorized/, 'Returned HTTP 401, invalid token.'); +like($response, qr/WWW-Authenticate: Bearer, error=\"invalid_token\"/, 'Return invalid_token challenge.'); +like($response, qr/Content-Type: application\/json/i, + 'Invalid token returned application/json body'); +like($response, qr/JWT validation failed: BAD_FORMAT/i, 'JWT error text in the body.'); + +my $bookstore_requests = $t->read_file('bookstore.log'); +is($bookstore_requests, '', 'Request did not reach the backend.'); + + +################################################################################ + +sub bookstore { + my ($t, $port, $file) = @_; + my $server = HttpServer->new($port, $t->testdir() . '/' . $file) + or die "Can't create test server socket: $!\n"; + local $SIG{PIPE} = 'IGNORE'; + # Do not initialize any server state, requests won't reach backend anyway. + $server->run(); +} + +################################################################################ + +sub servicecontrol { + my ($t, $port, $file, $done) = @_; + my $server = HttpServer->new($port, $t->testdir() . '/' . $file) + or die "Can't create test server socket: $!\n"; + local $SIG{PIPE} = 'IGNORE'; + + $server->on_sub('POST', '/v1/services/endpoints-test.cloudendpointsapis.com:report', sub { + my ($headers, $body, $client) = @_; + print $client <<'EOF'; +HTTP/1.1 200 OK +Connection: close + +{} +EOF + }); + + $server->run(); +} + +################################################################################ From 18f9a0b6bc93aacd60afafb1a76467a66b57cda4 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Tue, 1 Aug 2017 12:29:37 -0700 Subject: [PATCH 3/4] Added t auth_redirect test. --- src/nginx/error.cc | 9 ++++- src/nginx/t/Auth.pm | 6 +++ src/nginx/t/BUILD | 1 + src/nginx/t/auth_redirect.t | 80 ++++++++++++++----------------------- 4 files changed, 45 insertions(+), 51 deletions(-) diff --git a/src/nginx/error.cc b/src/nginx/error.cc index b1776ac80..512744eca 100644 --- a/src/nginx/error.cc +++ b/src/nginx/error.cc @@ -51,6 +51,11 @@ const u_char kLocationLowcase[] = "location"; ngx_str_t missing_credential = ngx_string("Bearer"); ngx_str_t invalid_token = ngx_string("Bearer, error=\"invalid_token\""); +const char *kInvalidAuthToken = + "JWT validation failed: Missing or invalid credentials"; +const char *kExpiredAuthToken = + "JWT validation failed: TIME_CONSTRAINT_FAILURE"; + ngx_http_output_header_filter_pt ngx_http_next_header_filter; ngx_http_output_body_filter_pt ngx_http_next_body_filter; @@ -108,7 +113,9 @@ ngx_int_t ngx_esp_handle_www_authenticate(ngx_http_request_t *r, ngx_int_t ngx_esp_handle_authorization_url(ngx_http_request_t *r, ngx_esp_request_ctx_t *ctx) { if (ctx && ctx->status.code() == Code::UNAUTHENTICATED && - ctx->status.error_cause() == utils::Status::AUTH) { + ctx->status.error_cause() == utils::Status::AUTH && + (ctx->status.message() == kInvalidAuthToken || + ctx->status.message() == kExpiredAuthToken)) { std::string url = ctx->request_handler->GetAuthorizationUrl(); if (!url.empty()) { r->headers_out.status = NGX_HTTP_MOVED_TEMPORARILY; diff --git a/src/nginx/t/Auth.pm b/src/nginx/t/Auth.pm index 45a8d4892..0d87cd528 100644 --- a/src/nginx/t/Auth.pm +++ b/src/nginx/t/Auth.pm @@ -86,4 +86,10 @@ sub get_public_key_x509 { EOF } +sub get_expired_jwt_token { + return <<'EOF'; +eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6ImIzMzE5YTE0NzUxNGRmN2VlNWU0YmNkZWU1MTM1MGNjODkwY2M4OWUifQ==.eyJpc3MiOiI2Mjg2NDU3NDE4ODEtbm9hYml1MjNmNWE4bThvdmQ4dWN2Njk4bGo3OHZ2MGxAZGV2ZWxvcGVyLmdzZXJ2aWNlYWNjb3VudC5jb20iLCJzdWIiOiI2Mjg2NDU3NDE4ODEtbm9hYml1MjNmNWE4bThvdmQ4dWN2Njk4bGo3OHZ2MGxAZGV2ZWxvcGVyLmdzZXJ2aWNlYWNjb3VudC5jb20iLCJhdWQiOiJlbmRwb2ludHMtdGVzdC5jbG91ZGVuZHBvaW50c2FwaXMuY29tIiwiaWF0IjoxNTAxNjA3NzYyLCJleHAiOjE1MDE2MTEzNjJ9.JSM5JncxrTf6RkGTybeRYDgK2SL1cjYs6H-lXYQYChp84YA_HBri38shQ_rBBmdJOHLoz3OQNcoI8CJjIs6ulIMT5uWJqyFW6eA9nonKhMPHUoqx6tpEvoLx6iClvP8IqEwgTPZi4YFdLvX7pMpVKbRK62Fnlun4CTus_M3exXMg6O5dqXr3j05gpeESTsquUfL75eJchFg8vmOo4M6hNy2hjn_6czwqvqNGAXQmDcTR2LkwzFYJByjM2xouxxH4x2nMT3-bbP6gPeqTfx0ZqDP5GDegZqVKdM7EImXwvhHowx0QW8HZQTMrXTuL5ENQAn-6XZaMR5mXxtQXJ3xLuw== +EOF +} + 1; diff --git a/src/nginx/t/BUILD b/src/nginx/t/BUILD index 0bb10063f..1cd86f9f3 100644 --- a/src/nginx/t/BUILD +++ b/src/nginx/t/BUILD @@ -53,6 +53,7 @@ nginx_suite( "auth_ok_check_fail.t", "auth_pass_user_info.t", "auth_pkey_cache.t", + "auth_redirect.t", "auth_unreachable_pkey.t", "new_http.t", "service_control_disabled.t", diff --git a/src/nginx/t/auth_redirect.t b/src/nginx/t/auth_redirect.t index 04aba7229..ed9df84ce 100644 --- a/src/nginx/t/auth_redirect.t +++ b/src/nginx/t/auth_redirect.t @@ -32,6 +32,7 @@ use warnings; use src::nginx::t::ApiManager; # Must be first (sets up import path to the Nginx test module) use src::nginx::t::HttpServer; use src::nginx::t::ServiceControl; +use src::nginx::t::Auth; use Test::Nginx; # Imports Nginx's test module use Test::More; # And the test framework @@ -40,11 +41,8 @@ use Test::More; # And the test framework # Port allocations my $NginxPort = ApiManager::pick_port(); -my $BackendPort = ApiManager::pick_port(); -my $ServiceControlPort = ApiManager::pick_port(); -my $RedirectPort = ApiManager::pick_port(); -my $t = Test::Nginx->new()->has(qw/http proxy/)->plan(11); +my $t = Test::Nginx->new()->has(qw/http proxy/)->plan(5); my $config = ApiManager::get_bookstore_service_config .<<"EOF"; producer_project_id: "endpoints-test" @@ -53,7 +51,7 @@ authentication { id: "test_auth" issuer: "628645741881-noabiu23f5a8m8ovd8ucv698lj78vv0l\@developer.gserviceaccount.com" jwks_uri: "http://127.0.0.1/pubkey" - authorization_url: "http://127.0.0.1:${RedirectPort}/redirect" + authorization_url: "http://dummy-redirect-url" } rules { selector: "ListShelves" @@ -64,7 +62,7 @@ authentication { } } control { - environment: "http://127.0.0.1:${ServiceControlPort}" + environment: "http://127.0.0.1:3000" } EOF $t->write_file('service.pb.txt', $config); @@ -86,65 +84,47 @@ http { api service.pb.txt; on; } - proxy_pass http://127.0.0.1:${BackendPort}; + proxy_pass http://127.0.0.1:3000; } } } EOF -$t->run_daemon(\&bookstore, $t, $BackendPort, 'bookstore.log'); -$t->run_daemon(\&servicecontrol, $t, $ServiceControlPort, 'servicecontrol.log', $report_done); - -is($t->waitforsocket("127.0.0.1:${BackendPort}"), 1, 'Bookstore socket ready.'); -is($t->waitforsocket("127.0.0.1:${ServiceControlPort}"), 1, 'Service control socket ready.'); - $t->run(); ################################################################################ -my $response = ApiManager::http_get($NginxPort, "/shelves"); - -$t->stop_daemons(); +# no auth token +my $response1 = ApiManager::http_get($NginxPort, "/shelves"); -like($response, qr/HTTP\/1\.1 401 Unauthorized/, 'Returned HTTP 401, invalid token.'); -like($response, qr/WWW-Authenticate: Bearer, error=\"invalid_token\"/, 'Return invalid_token challenge.'); -like($response, qr/Content-Type: application\/json/i, - 'Invalid token returned application/json body'); -like($response, qr/JWT validation failed: BAD_FORMAT/i, 'JWT error text in the body.'); +# should redirect +like($response1, qr/HTTP\/1\.1 302 Moved Temporarily/, 'Returned HTTP 302.'); +like($response1, qr/Location: http:\/\/dummy-redirect-url/, 'Return correct redirect location.'); -my $bookstore_requests = $t->read_file('bookstore.log'); -is($bookstore_requests, '', 'Request did not reach the backend.'); +# expired auth token +my $expired_token = Auth::get_expired_jwt_token; +my $response2 = ApiManager::http($NginxPort, <<"EOF"); +GET /shelves HTTP/1.0 +Host: localhost +Authorization: Bearer $expired_token +EOF -################################################################################ - -sub bookstore { - my ($t, $port, $file) = @_; - my $server = HttpServer->new($port, $t->testdir() . '/' . $file) - or die "Can't create test server socket: $!\n"; - local $SIG{PIPE} = 'IGNORE'; - # Do not initialize any server state, requests won't reach backend anyway. - $server->run(); -} - -################################################################################ - -sub servicecontrol { - my ($t, $port, $file, $done) = @_; - my $server = HttpServer->new($port, $t->testdir() . '/' . $file) - or die "Can't create test server socket: $!\n"; - local $SIG{PIPE} = 'IGNORE'; +# should redirect +like($response2, qr/HTTP\/1\.1 302 Moved Temporarily/, 'Returned HTTP 302.'); +like($response2, qr/Location: http:\/\/dummy-redirect-url/, 'Return correct redirect location.'); - $server->on_sub('POST', '/v1/services/endpoints-test.cloudendpointsapis.com:report', sub { - my ($headers, $body, $client) = @_; - print $client <<'EOF'; -HTTP/1.1 200 OK -Connection: close +#invalid auth token +my $response3 = ApiManager::http($NginxPort, <<"EOF"); +GET /shelves HTTP/1.0 +Host: localhost +Authorization: Bearer invalid_token -{} EOF - }); - $server->run(); -} +# should not redirect. +like($response3, qr/HTTP\/1\.1 401 Unauthorized/, 'Returned HTTP 401.'); + +$t->stop_daemons(); ################################################################################ + From e093d7a0ae43584e171af72f6cfe902c31c16e75 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Tue, 1 Aug 2017 12:52:19 -0700 Subject: [PATCH 4/4] fix test. --- src/grpc/transcoding/transcoder_test.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/grpc/transcoding/transcoder_test.cc b/src/grpc/transcoding/transcoder_test.cc index 00c466119..7bfee6679 100644 --- a/src/grpc/transcoding/transcoder_test.cc +++ b/src/grpc/transcoding/transcoder_test.cc @@ -112,6 +112,16 @@ class TestMethodInfo : public MethodInfo { bool response_streaming() const { return response_streaming_; } const std::string &body_field_path() const { return body_field_path_; } + const std::string &authorization_url_by_issuer( + const std::string &issuer) const { + static std::string empty; + return empty; + } + const std::string &first_authorization_url() const { + static std::string empty; + return empty; + } + private: std::string request_type_url_; std::string response_type_url_;