From 621211d1c72e9f819b248cb8b8c0410447545603 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Tue, 5 Jan 2021 15:27:56 -0800 Subject: [PATCH] remove authorizationUrl redirect (#824) Signed-off-by: Wayne Zhang --- src/nginx/error.cc | 43 ------------ src/nginx/t/BUILD | 1 - src/nginx/t/auth_redirect.t | 130 ------------------------------------ 3 files changed, 174 deletions(-) delete mode 100644 src/nginx/t/auth_redirect.t diff --git a/src/nginx/error.cc b/src/nginx/error.cc index 06db96ca6..18f717e58 100644 --- a/src/nginx/error.cc +++ b/src/nginx/error.cc @@ -47,16 +47,9 @@ 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\""); -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; @@ -109,39 +102,6 @@ ngx_int_t ngx_esp_handle_www_authenticate(ngx_http_request_t *r, 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 && - (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; - - 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)); @@ -170,9 +130,6 @@ ngx_int_t ngx_esp_error_header_filter(ngx_http_request_t *r) { ngx_int_t ret; ret = ngx_esp_handle_www_authenticate(r, ctx); if (ret != NGX_OK) return ret; - - 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/BUILD b/src/nginx/t/BUILD index 4e51b46ca..d6a1f50d6 100644 --- a/src/nginx/t/BUILD +++ b/src/nginx/t/BUILD @@ -57,7 +57,6 @@ nginx_suite( "auth_ok_check_fail.t", "auth_pass_user_info.t", "auth_pkey_cache.t", - "auth_redirect.t", "auth_remove_user_info.t", "auth_unreachable_pkey.t", "new_http.t", diff --git a/src/nginx/t/auth_redirect.t b/src/nginx/t/auth_redirect.t deleted file mode 100644 index ed9df84ce..000000000 --- a/src/nginx/t/auth_redirect.t +++ /dev/null @@ -1,130 +0,0 @@ -# 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 src::nginx::t::Auth; -use Test::Nginx; # Imports Nginx's test module -use Test::More; # And the test framework - -################################################################################ - -# Port allocations - -my $NginxPort = ApiManager::pick_port(); - -my $t = Test::Nginx->new()->has(qw/http proxy/)->plan(5); - -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://dummy-redirect-url" - } - rules { - selector: "ListShelves" - requirements { - provider_id: "test_auth" - audiences: "ok_audience_1" - } - } -} -control { - environment: "http://127.0.0.1:3000" -} -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:3000; - } - } -} -EOF - -$t->run(); - -################################################################################ -# no auth token -my $response1 = ApiManager::http_get($NginxPort, "/shelves"); - -# 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.'); - -# 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 - -# 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.'); - -#invalid auth token -my $response3 = ApiManager::http($NginxPort, <<"EOF"); -GET /shelves HTTP/1.0 -Host: localhost -Authorization: Bearer invalid_token - -EOF - -# should not redirect. -like($response3, qr/HTTP\/1\.1 401 Unauthorized/, 'Returned HTTP 401.'); - -$t->stop_daemons(); - -################################################################################ -