mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-19 03:50:54 +01:00
Improve error message for Conduit path problems
Summary: A few people in IRC have been having issues here recently. If you misconfigure the IRC bot, e.g., you get a 200 response back with a bunch of login HTML in it. This is unhelpful. Try to detect that a conduit request is going to the wrong path and raise a concise, explicit error which is comprehensible from the CLI. Also created a "PlainText" response and moved the IE nosniff header to the base response object. Test Plan: As a logged-out user, hit various nonsense with "?__conduit__=true" in the URI. Got good error messages. Hit nonsense without it, got login screens. Reviewers: btrahan, jungejason Reviewed By: btrahan CC: aran Maniphest Tasks: T775 Differential Revision: https://secure.phabricator.com/D1407
This commit is contained in:
parent
f109342a7a
commit
f81021fa7f
8 changed files with 90 additions and 7 deletions
|
@ -58,6 +58,7 @@ phutil_register_library_map(array(
|
||||||
'AphrontPageView' => 'view/page/base',
|
'AphrontPageView' => 'view/page/base',
|
||||||
'AphrontPagerView' => 'view/control/pager',
|
'AphrontPagerView' => 'view/control/pager',
|
||||||
'AphrontPanelView' => 'view/layout/panel',
|
'AphrontPanelView' => 'view/layout/panel',
|
||||||
|
'AphrontPlainTextResponse' => 'aphront/response/plaintext',
|
||||||
'AphrontQueryAccessDeniedException' => 'storage/exception/accessdenied',
|
'AphrontQueryAccessDeniedException' => 'storage/exception/accessdenied',
|
||||||
'AphrontQueryConnectionException' => 'storage/exception/connection',
|
'AphrontQueryConnectionException' => 'storage/exception/connection',
|
||||||
'AphrontQueryConnectionLostException' => 'storage/exception/connectionlost',
|
'AphrontQueryConnectionLostException' => 'storage/exception/connectionlost',
|
||||||
|
@ -830,6 +831,7 @@ phutil_register_library_map(array(
|
||||||
'AphrontPageView' => 'AphrontView',
|
'AphrontPageView' => 'AphrontView',
|
||||||
'AphrontPagerView' => 'AphrontView',
|
'AphrontPagerView' => 'AphrontView',
|
||||||
'AphrontPanelView' => 'AphrontView',
|
'AphrontPanelView' => 'AphrontView',
|
||||||
|
'AphrontPlainTextResponse' => 'AphrontResponse',
|
||||||
'AphrontQueryAccessDeniedException' => 'AphrontQueryRecoverableException',
|
'AphrontQueryAccessDeniedException' => 'AphrontQueryRecoverableException',
|
||||||
'AphrontQueryConnectionException' => 'AphrontQueryException',
|
'AphrontQueryConnectionException' => 'AphrontQueryException',
|
||||||
'AphrontQueryConnectionLostException' => 'AphrontQueryRecoverableException',
|
'AphrontQueryConnectionLostException' => 'AphrontQueryRecoverableException',
|
||||||
|
|
|
@ -26,6 +26,7 @@ class AphrontRequest {
|
||||||
|
|
||||||
const TYPE_AJAX = '__ajax__';
|
const TYPE_AJAX = '__ajax__';
|
||||||
const TYPE_FORM = '__form__';
|
const TYPE_FORM = '__form__';
|
||||||
|
const TYPE_CONDUIT = '__conduit__';
|
||||||
|
|
||||||
private $host;
|
private $host;
|
||||||
private $path;
|
private $path;
|
||||||
|
@ -170,6 +171,10 @@ class AphrontRequest {
|
||||||
return $this->getExists(self::TYPE_AJAX);
|
return $this->getExists(self::TYPE_AJAX);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
final public function isConduit() {
|
||||||
|
return $this->getExists(self::TYPE_CONDUIT);
|
||||||
|
}
|
||||||
|
|
||||||
public static function getCSRFTokenName() {
|
public static function getCSRFTokenName() {
|
||||||
return '__csrf__';
|
return '__csrf__';
|
||||||
}
|
}
|
||||||
|
|
|
@ -1,7 +1,7 @@
|
||||||
<?php
|
<?php
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Copyright 2011 Facebook, Inc.
|
* Copyright 2012 Facebook, Inc.
|
||||||
*
|
*
|
||||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
* you may not use this file except in compliance with the License.
|
* you may not use this file except in compliance with the License.
|
||||||
|
@ -91,6 +91,14 @@ abstract class AphrontResponse {
|
||||||
$this->formatEpochTimestampForHTTPHeader($this->lastModified));
|
$this->formatEpochTimestampForHTTPHeader($this->lastModified));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
$headers[] = array(
|
||||||
|
// IE has a feature where it may override an explicit Content-Type
|
||||||
|
// declaration by inferring a content type. This can be a security risk
|
||||||
|
// and we always explicitly transmit the correct Content-Type header, so
|
||||||
|
// prevent IE from using inferred content types.
|
||||||
|
array('X-Content-Type-Options', 'nosniff'),
|
||||||
|
);
|
||||||
|
|
||||||
return $headers;
|
return $headers;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -1,7 +1,7 @@
|
||||||
<?php
|
<?php
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Copyright 2011 Facebook, Inc.
|
* Copyright 2012 Facebook, Inc.
|
||||||
*
|
*
|
||||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
* you may not use this file except in compliance with the License.
|
* you may not use this file except in compliance with the License.
|
||||||
|
@ -59,11 +59,6 @@ class AphrontFileResponse extends AphrontResponse {
|
||||||
public function getHeaders() {
|
public function getHeaders() {
|
||||||
$headers = array(
|
$headers = array(
|
||||||
array('Content-Type', $this->getMimeType()),
|
array('Content-Type', $this->getMimeType()),
|
||||||
// Without this, IE can decide that we surely meant "text/html" when
|
|
||||||
// delivering another content type since, you know, it looks like it's
|
|
||||||
// probably an HTML document. This closes the security hole that policy
|
|
||||||
// creates.
|
|
||||||
array('X-Content-Type-Options', 'nosniff'),
|
|
||||||
);
|
);
|
||||||
|
|
||||||
if (strlen($this->getDownload())) {
|
if (strlen($this->getDownload())) {
|
||||||
|
|
41
src/aphront/response/plaintext/AphrontPlainTextResponse.php
Normal file
41
src/aphront/response/plaintext/AphrontPlainTextResponse.php
Normal file
|
@ -0,0 +1,41 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Copyright 2012 Facebook, Inc.
|
||||||
|
*
|
||||||
|
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
|
* you may not use this file except in compliance with the License.
|
||||||
|
* You may obtain a copy of the License at
|
||||||
|
*
|
||||||
|
* http://www.apache.org/licenses/LICENSE-2.0
|
||||||
|
*
|
||||||
|
* Unless required by applicable law or agreed to in writing, software
|
||||||
|
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||||
|
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||||
|
* See the License for the specific language governing permissions and
|
||||||
|
* limitations under the License.
|
||||||
|
*/
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @group aphront
|
||||||
|
*/
|
||||||
|
class AphrontPlainTextResponse extends AphrontResponse {
|
||||||
|
|
||||||
|
public function setContent($content) {
|
||||||
|
$this->content = $content;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function buildResponseString() {
|
||||||
|
return $this->content;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getHeaders() {
|
||||||
|
$headers = array(
|
||||||
|
array('Content-Type', 'text/plain'),
|
||||||
|
);
|
||||||
|
|
||||||
|
return array_merge(parent::getHeaders(), $headers);
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
12
src/aphront/response/plaintext/__init__.php
Normal file
12
src/aphront/response/plaintext/__init__.php
Normal file
|
@ -0,0 +1,12 @@
|
||||||
|
<?php
|
||||||
|
/**
|
||||||
|
* This file is automatically generated. Lint this module to rebuild it.
|
||||||
|
* @generated
|
||||||
|
*/
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
phutil_require_module('phabricator', 'aphront/response/base');
|
||||||
|
|
||||||
|
|
||||||
|
phutil_require_source('AphrontPlainTextResponse.php');
|
|
@ -30,6 +30,25 @@ class PhabricatorLoginController extends PhabricatorAuthController {
|
||||||
return id(new AphrontRedirectResponse())->setURI('/');
|
return id(new AphrontRedirectResponse())->setURI('/');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if ($request->isConduit()) {
|
||||||
|
|
||||||
|
// A common source of errors in Conduit client configuration is getting
|
||||||
|
// the request path wrong. The client will end up here, so make some
|
||||||
|
// effort to give them a comprehensible error message.
|
||||||
|
|
||||||
|
$request_path = $this->getRequest()->getPath();
|
||||||
|
$conduit_path = '/api/<method>';
|
||||||
|
$example_path = '/api/conduit.ping';
|
||||||
|
|
||||||
|
$message =
|
||||||
|
"ERROR: You are making a Conduit API request to '{$request_path}', ".
|
||||||
|
"but the correct HTTP request path to use in order to access a ".
|
||||||
|
"Conduit method is '{$conduit_path}' (for example, ".
|
||||||
|
"'{$example_path}'). Check your configuration.";
|
||||||
|
|
||||||
|
return id(new AphrontPlainTextResponse())->setContent($message);
|
||||||
|
}
|
||||||
|
|
||||||
$next_uri = $this->getRequest()->getPath();
|
$next_uri = $this->getRequest()->getPath();
|
||||||
if ($next_uri == '/login/') {
|
if ($next_uri == '/login/') {
|
||||||
$next_uri = '/';
|
$next_uri = '/';
|
||||||
|
|
|
@ -6,6 +6,7 @@
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
phutil_require_module('phabricator', 'aphront/response/plaintext');
|
||||||
phutil_require_module('phabricator', 'aphront/response/redirect');
|
phutil_require_module('phabricator', 'aphront/response/redirect');
|
||||||
phutil_require_module('phabricator', 'applications/auth/controller/base');
|
phutil_require_module('phabricator', 'applications/auth/controller/base');
|
||||||
phutil_require_module('phabricator', 'applications/auth/oauth/provider/base');
|
phutil_require_module('phabricator', 'applications/auth/oauth/provider/base');
|
||||||
|
|
Loading…
Reference in a new issue