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.

Sign up for GitHub

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

Add cookie strategy configuration to PHP request handler #1753

Open
wants to merge 9 commits into
base: trunk
Choose a base branch
from
Open

Add cookie strategy configuration to PHP request handler #1753

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e6ee1b7
Add cookie strategy to PHP request handler configuration
fluiddot Sep 11, 2024
934d1be
Add unit tests to cover cookie strategy logic
fluiddot Sep 11, 2024
f77d0c5
Add cookie strategy to boot function
fluiddot Sep 11, 2024
e532d08
Use `pass-through` cookie strategy in Playground CLI
fluiddot Sep 11, 2024
4938594
Add mechanism to allow auto-login in Playground CLI
fluiddot Sep 11, 2024
985fe83
Merge branch 'trunk' into update/allow-cookie-configuration
fluiddot Sep 11, 2024
ef8e39e
Update description of `cookieStrategy` option
fluiddot Sep 12, 2024
364e7de
Add unit test to cover not passing cookie strategy
fluiddot Sep 12, 2024
3ab3876
Move `cookieStrategy` description to type definition
fluiddot Sep 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 81 additions & 0 deletions packages/php-wasm/node/src/test/php-request-handler.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -765,3 +765,84 @@ describe('PHPRequestHandler – Loopback call', () => {
expect(response.text).toEqual('Starting: Ran second.php! Done');
});
});

describe('PHPRequestHandler – Cookie strategy', () => {
it('should persist cookies internally with internal-store strategy', async () => {
const handler = new PHPRequestHandler({
documentRoot: '/',
phpFactory: async () =>
new PHP(await loadNodeRuntime(RecommendedPHPVersion)),
maxPhpInstances: 1,
fluiddot marked this conversation as resolved.
Show resolved Hide resolved
});
const php = await handler.getPrimaryPhp();

php.writeFile(
'/set-cookie.php',
`<?php setcookie("my-cookie", "where-is-my-cookie", time() + 3600, "/");`
);
php.writeFile('/get-cookie.php', `<?php echo json_encode($_COOKIE);`);

// Cookies return in the response
let response = await handler.request({
url: '/set-cookie.php',
});
const cookies = response.headers['set-cookie'];
expect(cookies).toHaveLength(1);
expect(cookies[0]).toMatch(
/my-cookie=where-is-my-cookie; expires=.*; Max-Age=3600; path=\//
);

// Cookies are persisted internally in the request handler.
// Note that we are not passing cookies in the header of the response.
response = await handler.request({
url: '/get-cookie.php',
});
expect(response.text).toEqual(
JSON.stringify({ 'my-cookie': 'where-is-my-cookie' })
);
});

it('should not persist cookies internally with pass-through strategy', async () => {
const handler = new PHPRequestHandler({
documentRoot: '/',
phpFactory: async () =>
new PHP(await loadNodeRuntime(RecommendedPHPVersion)),
maxPhpInstances: 1,
cookieStrategy: 'pass-through',
});
const php = await handler.getPrimaryPhp();

php.writeFile(
'/set-cookie.php',
`<?php setcookie("my-cookie", "where-is-my-cookie", time() + 3600, "/");`
);
php.writeFile('/get-cookie.php', `<?php echo json_encode($_COOKIE);`);

// Cookies return in the response
let response = await handler.request({
url: '/set-cookie.php',
});
const cookies = response.headers['set-cookie'];
expect(cookies).toHaveLength(1);
expect(cookies[0]).toMatch(
/my-cookie=where-is-my-cookie; expires=.*; Max-Age=3600; path=\//
);

// No cookies are persisted internally.
// Note that we are not passing cookies in the header of the response.
response = await handler.request({
url: '/get-cookie.php',
});
expect(response.text).toEqual(JSON.stringify([]));

// Cookies are available in the PHP environment when passed in the
// request.
response = await handler.request({
url: '/get-cookie.php',
headers: { Cookie: 'my-cookie=where-is-my-cookie' },
});
expect(response.text).toEqual(
JSON.stringify({ 'my-cookie': 'where-is-my-cookie' })
);
});
});
1 change: 1 addition & 0 deletions packages/php-wasm/universal/src/lib/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export type {
} from './load-php-runtime';

export type {
CookieStrategy,
PHPRequestHandlerConfiguration,
RewriteRule,
} from './php-request-handler';
Expand Down
31 changes: 26 additions & 5 deletions packages/php-wasm/universal/src/lib/php-request-handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ export type FileNotFoundGetActionCallback = (
relativePath: string
) => FileNotFoundAction;

export type CookieStrategy = 'internal-store' | 'pass-through';

interface BaseConfiguration {
/**
* The directory in the PHP filesystem where the server will look
Expand Down Expand Up @@ -95,7 +97,20 @@ export type PHPRequestHandlerConfiguration = BaseConfiguration &
*/
maxPhpInstances?: number;
}
);
) & {
/**
* - `internal-store`: Persist cookies from reponses in an internal store and
* includen them in following requests. This is a behavior mostly needed when
* using Playground in web.
*
* - `pass-through`: Avoid persisting cookies internally and let them pass
* through the requests and back to the caller from responses. This is the
* common behavior in requests handled by a server.
*
* Default value is `internal-store`.
*/
cookieStrategy?: CookieStrategy;
};

/**
* Handles HTTP requests using PHP runtime as a backend.
Expand Down Expand Up @@ -159,7 +174,7 @@ export class PHPRequestHandler {
#HOST: string;
#PATHNAME: string;
#ABSOLUTE_URL: string;
#cookieStore: HttpCookieStore;
#cookieStore?: HttpCookieStore;
rewriteRules: RewriteRule[];
processManager: PHPProcessManager;
getFileNotFoundAction: FileNotFoundGetActionCallback;
Expand Down Expand Up @@ -198,7 +213,10 @@ export class PHPRequestHandler {
maxPhpInstances: config.maxPhpInstances,
});
}
this.#cookieStore = new HttpCookieStore();
const cookieStrategy = config.cookieStrategy ?? 'internal-store';
if (cookieStrategy === 'internal-store') {
this.#cookieStore = new HttpCookieStore();
}
this.#DOCROOT = documentRoot;

const url = new URL(absoluteUrl);
Expand Down Expand Up @@ -492,9 +510,12 @@ export class PHPRequestHandler {
const headers: Record<string, string> = {
host: this.#HOST,
...normalizeHeaders(request.headers || {}),
cookie: this.#cookieStore.getCookieRequestHeader(),
};

if (this.#cookieStore) {
headers['cookie'] = this.#cookieStore.getCookieRequestHeader();
}

let body = request.body;
if (typeof body === 'object' && !(body instanceof Uint8Array)) {
preferredMethod = 'POST';
Expand Down Expand Up @@ -522,7 +543,7 @@ export class PHPRequestHandler {
scriptPath,
headers,
});
this.#cookieStore.rememberCookiesFromResponseHeaders(
this.#cookieStore?.rememberCookiesFromResponseHeaders(
response.headers
);
return response;
Expand Down
7 changes: 6 additions & 1 deletion packages/playground/cli/src/cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,6 @@ async function run() {
php: args.php as SupportedPHPVersion,
wp: args.wp,
},
login: args.login,
};
}

Expand Down Expand Up @@ -311,6 +310,7 @@ async function run() {
}
},
},
cookieStrategy: 'pass-through',
});

const php = await requestHandler.getPrimaryPhp();
Expand Down Expand Up @@ -348,6 +348,11 @@ async function run() {
process.exit(0);
} else {
logger.log(`WordPress is running on ${absoluteUrl}`);
if (args.login) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm starting to think we should just ship a WordPress plugin to log the user in the first time they interact with the site. It could go like this:

  1. The login step would install that mu-plugin. Or maybe the plugin would always be there and login would just set some constant to activate it?
  2. The plugin would check for a cookie like wordpress_user_was_auto_logged. If it's present, we're done. If it's missing, set it and also log the user in.

This should work in all runtimes (Studio, Playground CLI, WP-NOW, Playground webapp) without any runtime-specific loginc.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Claude gave me this. Coming from AI, there's likely some problem with that code, but it seems like a good starting point:

<?php
/*
Plugin Name: Auto Admin Login
Description: Automatically logs in as admin if a specific cookie is not present
Version: 1.0
Author: Your Name
*/

// Function to check cookie and perform auto-login
function auto_admin_login() {
    $cookie_name = 'wordpress_user_was_auto_logged';

    // Check if the cookie exists
    if (isset($_COOKIE[$cookie_name])) {
        return;
    }

    // Cookie doesn't exist, set it
    setcookie($cookie_name, '1', time() + (86400 * 30), '/'); // Cookie expires in 30 days

    // Get the admin user
    $admin_user = get_user_by('login', 'admin');

    // If admin user exists, log them in
    if ($admin_user) {
        wp_set_current_user($admin_user->ID);
        wp_set_auth_cookie($admin_user->ID);
        do_action('wp_login', $admin_user->user_login, $admin_user);
    }
}

// Hook the function to run on WordPress init
add_action('init', 'auto_admin_login');

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, good idea. However, I'm concerned about using a plugin in Studio's case, since we'd need to filter it out when exporting the site.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh I don't want to pollute the exports, filtering files out just doesn't work as a long term strategy. The login plugin would live in /internal/shared/mu-plugins with the rest of the platform-level mu-plugins to keep the site clean.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The login plugin would live in /internal/shared/mu-plugins with the rest of the platform-level mu-plugins to keep the site clean.

Ah, ok, I didn't know about this approach 😅.

Still, I wonder about how we could make this plugin work so the user can decide when it should auto-login and when not. If I understand correctly the code shared here, the init hook will make that all requests are logged, right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@fluiddot here's my thinking:

  • The runtime (Studio, Playground webapp etc.) decides whether or not the plugin is installed at any given time. The plugin is easy to install and easy to remove – it's just a php file.
  • When the plugin is in place, it automatically logs the user in as an admin, or as another user when a constant / env variable like AUTOLOGIN_USERNAME is present.
  • Autologin sets auth cookies valid for a year, and an extra cookie called, say, autologin_performed
  • If the autologin_performed cookie is already set, no autologin happens.

This way the user will remain logged in until they explicitly log out, at which point they'll remain logged out until they explicitly log in. A private browsing session will still default to the logged in state.

There could also be a "querystring mode" where autologin is only performed when $_GET['playground-autologin'] is present. This would give Studio a link-based control while still allowing the Playground webapp and CLI to default to autologin.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The approach looks good to me, thanks @adamziel ! I'll try to work on this in the next few days. Note that the original issue lowered its priority, so I might delay updating the PR.

There could also be a "querystring mode" where autologin is only performed when $_GET['playground-autologin'] is present. This would give Studio a link-based control while still allowing the Playground webapp and CLI to default to autologin.

At least for the Studio app, and probably when using node clients, this will be necessary to ensure that API requests to the site are not auto-logged. This is important when using, for instance, API keys as referenced in Automattic/studio#387.

Copy link
Collaborator

@adamziel adamziel Sep 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

A check like if(!$ajax && !$rest_api) could also help solve for the API requests. Anyway, whatever's most useful here.

logger.log(
`➜ You can auto-login to the site using the query parameter "playground-auto-login=true"\n➜ Homepage: ${absoluteUrl}/?playground-auto-login=true\n➜ WP-Admin: ${absoluteUrl}/wp-admin/?playground-auto-login=true`
);
}
}
},
async handleRequest(request: PHPRequest) {
Expand Down
29 changes: 29 additions & 0 deletions packages/playground/cli/src/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,35 @@ export async function startServer(options: ServerOptions) {
});
});

// Middleware to check if auto-login should be executed
app.use(async (req, res, next) => {
if (req.query['playground-auto-login'] === 'true') {
await options.handleRequest({ url: '/wp-login.php' });
const response = await options.handleRequest({
url: '/wp-login.php',
method: 'POST',
body: {
log: 'admin',
pwd: 'password',
rememberme: 'forever',
},
});
const cookies = response.headers['set-cookie'];
res.setHeader('set-cookie', cookies);
// Remove query parameter to avoid infinite loop
let redirectUrl = req.url.replace(
/&?playground-auto-login=true/,
''
);
// If no more query parameters, remove ? from URL
if (Object.keys(req.query).length === 1) {
redirectUrl = redirectUrl.substring(0, redirectUrl.length - 1);
}
return res.redirect(redirectUrl);
}
next();
});

app.use('/', async (req, res) => {
const phpResponse = await options.handleRequest({
url: req.url,
Expand Down
15 changes: 15 additions & 0 deletions packages/playground/wordpress/src/boot.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
CookieStrategy,
FileNotFoundAction,
FileNotFoundGetActionCallback,
FileTree,
Expand Down Expand Up @@ -91,6 +92,19 @@ export interface BootOptions {
* given request URI.
*/
getFileNotFoundAction?: FileNotFoundGetActionCallback;

/**
* - `internal-store`: Persist cookies from reponses in an internal store and
* includen them in following requests. This is a behavior mostly needed when
* using Playground in web.
*
* - `pass-through`: Avoid persisting cookies internally and let them pass
* through the requests and back to the caller from responses. This is the
* common behavior in requests handled by a server.
fluiddot marked this conversation as resolved.
Show resolved Hide resolved
*
* Default value is `internal-store`.
*/
cookieStrategy?: CookieStrategy;
}

/**
Expand Down Expand Up @@ -179,6 +193,7 @@ export async function bootWordPress(options: BootOptions) {
rewriteRules: wordPressRewriteRules,
getFileNotFoundAction:
options.getFileNotFoundAction ?? getFileNotFoundActionForWordPress,
cookieStrategy: options.cookieStrategy,
});

const php = await requestHandler.getPrimaryPhp();
Expand Down
Loading