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.

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

proposal(www): allow providers to make access keys shareable. #1836

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ GOBIND=env PATH="$(GOBIN):$(PATH)" "$(GOMOBILE)" bind
IMPORT_HOST=github.com
IMPORT_PATH=$(IMPORT_HOST)/Jigsaw-Code/outline-client

.PHONY: android apple linux windows
.PHONY: android apple linux windows browser

all: android apple linux windows

Expand Down Expand Up @@ -73,3 +73,6 @@ $(XGO): go.mod
go.mod: tools.go
go mod tidy
touch go.mod

browser:
echo 'browser environment: nothing to do'
5 changes: 5 additions & 0 deletions commitlint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,10 @@ module.exports = {
'service/windows',
],
],
'type-enum': [
2,
'always',
['build', 'chore', 'ci', 'docs', 'feat', 'fix', 'perf', 'proposal', 'refactor', 'revert', 'style', 'test'],
],
},
};
8 changes: 8 additions & 0 deletions resources/original_messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,14 @@
"description": "The success message after renaming a server in the application.",
"message": "Server renamed"
},
"server_share": {
"description": "The text of an option displayed in a server card's options menu to tell the application to share the server's access key with another application.",
"message": "Share"
},
"server_share_text": {
"description": "The text of a message that appears when the user clicks the Share option in a server card's options menu.",
"message": "This is an access key for an Outline Server. To use it, download the Outline app from the App Store or Google Play."
},
"servers_menu_item": {
"description": "The menu item text to navigate to the list of servers.",
"message": "Servers"
Expand Down
2 changes: 1 addition & 1 deletion src/electron/go_vpn_tunnel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {powerMonitor} from 'electron';
import {platform} from 'os';

import {pathToEmbeddedBinary} from '../infrastructure/electron/app_paths';
import {ShadowsocksSessionConfig} from '../www/app/tunnel';
import {ShadowsocksSessionConfig} from '../www/model/shadowsocks_session_config';
import {TunnelStatus} from '../www/app/tunnel';
import {ErrorCode, fromErrorCode, UnexpectedPluginError} from '../www/model/errors';

Expand Down
2 changes: 1 addition & 1 deletion src/electron/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import autoLaunch = require('auto-launch'); // tslint:disable-line

import * as errors from '../www/model/errors';

import {ShadowsocksSessionConfig} from '../www/app/tunnel';
import {ShadowsocksSessionConfig} from '../www/model/shadowsocks_session_config';
import {TunnelStatus} from '../www/app/tunnel';
import {GoVpnTunnel} from './go_vpn_tunnel';
import {installRoutingServices, RoutingDaemon} from './routing_service';
Expand Down
2 changes: 1 addition & 1 deletion src/electron/tunnel_store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import * as fs from 'fs';
import * as path from 'path';

import {ShadowsocksSessionConfig} from '../www/app/tunnel';
import {ShadowsocksSessionConfig} from '../www/model/shadowsocks_session_config';

// Format to store a tunnel configuration.
export interface SerializableTunnel {
Expand Down
44 changes: 43 additions & 1 deletion src/www/app/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@
this.rootEl.addEventListener('DisconnectPressed', this.disconnectServer.bind(this));
this.rootEl.addEventListener('ForgetPressed', this.forgetServer.bind(this));
this.rootEl.addEventListener('RenameRequested', this.renameServer.bind(this));
this.rootEl.addEventListener('ForgetPressed', this.forgetServer.bind(this));
this.rootEl.addEventListener('ShareServer', this.shareServer.bind(this));

Check warning on line 134 in src/www/app/app.ts

View check run for this annotation

Codecov / codecov/patch

src/www/app/app.ts#L133-L134

Added lines #L133 - L134 were not covered by tests
this.rootEl.addEventListener('QuitPressed', this.quitApplication.bind(this));
this.rootEl.addEventListener('AutoConnectDialogDismissed', this.autoConnectDialogDismissed.bind(this));
this.rootEl.addEventListener('ShowServerRename', this.rootEl.showServerRename.bind(this.rootEl));
Expand Down Expand Up @@ -378,6 +380,23 @@
this.serverRepo.rename(serverId, newName);
}

private async shareServer(event: CustomEvent) {

Check warning on line 383 in src/www/app/app.ts

View check run for this annotation

Codecov / codecov/patch

src/www/app/app.ts#L383

Added line #L383 was not covered by tests
const {serverId} = event.detail;
const server = this.getServerByServerId(serverId);

// TODO: fallback to copying to clipboard if share is not available

Check warning on line 387 in src/www/app/app.ts

View check run for this annotation

Codecov / codecov/patch

src/www/app/app.ts#L385-L387

Added lines #L385 - L387 were not covered by tests
if (!navigator.share) {
console.warn('Web Share API not available');
return;

Check warning on line 390 in src/www/app/app.ts

View check run for this annotation

Codecov / codecov/patch

src/www/app/app.ts#L389-L390

Added lines #L389 - L390 were not covered by tests
}

await navigator.share({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to test this on multiple platforms!

Copy link
Collaborator

Choose a reason for hiding this comment

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

What Chrome version are we using with Electron? Compare with https://caniuse.com/?search=navigator.share

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe it's over 100, we should be okay - but I will test to confirm once we're aligned on this idea

title: server.name || 'Outline Server',
text: this.localize('share-server-text'),
url: server.accessKey,
});
}

Check warning on line 399 in src/www/app/app.ts

View check run for this annotation

Codecov / codecov/patch

src/www/app/app.ts#L395-L399

Added lines #L395 - L399 were not covered by tests
private async connectServer(event: CustomEvent) {
event.stopImmediatePropagation();

Expand Down Expand Up @@ -589,7 +608,7 @@
// Helpers:

private makeServerListItem(server: Server): ServerListItem {
return {
const serverListItem: ServerListItem = {
disabled: false,
errorMessageId: server.errorMessageId,
isOutlineServer: server.isOutlineServer,
Expand All @@ -598,6 +617,29 @@
id: server.id,
connectionState: ServerConnectionState.DISCONNECTED,
};

if (server.sessionConfig?.extra) {
const extraParams = server.sessionConfig.extra;

if (['error', 'warning', 'info'].includes(extraParams.messageType) && extraParams.messageContent) {
serverListItem.message = {
type: extraParams.messageType as 'error' | 'warning' | 'info',
content: extraParams.messageContent,
};
}

if (extraParams.contactEmail) {
serverListItem.contact = {
email: extraParams.email,
};
}

Check warning on line 636 in src/www/app/app.ts

View check run for this annotation

Codecov / codecov/patch

src/www/app/app.ts#L636

Added line #L636 was not covered by tests
if (extraParams.share) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it should be called shareable, to indicate that the key is shareable. With ephemeral keys, access keys may not be shared.

Also, we could add this to the static keys as well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I do worry about extending our bad config format that doesn't differentiate service and transport configuration, and has a very rigid transport configuration. I would like us to not get stuck with it.
At a minimum we should have our own internal format that is more sensible and can be mapped from the serialized formats.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hear you, but I wonder the degree to which we're tied to SIPOO2 already? I mean people use the client to connect to other shadowsocks servers all the time.

How about this - if we don't block adding these features to the access keys on inventing a new format I promise to draft a proposal for an outline:// config that we can all iterate on. Then I'll replace ShadowsocksConfig with a less overengineered solution that also handles outline2sskey

Copy link
Contributor Author

Choose a reason for hiding this comment

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

serverListItem.canShare = true;
}

Check warning on line 639 in src/www/app/app.ts

View check run for this annotation

Codecov / codecov/patch

src/www/app/app.ts#L639

Added line #L639 was not covered by tests
}

return serverListItem;
}

private throttleServerConnectionChange(serverId: string, time: number) {
Expand Down
2 changes: 1 addition & 1 deletion src/www/app/cordova_main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import {Tunnel, TunnelStatus} from './tunnel';
import {AbstractUpdater} from './updater';
import * as interceptors from './url_interceptor';
import {FakeOutlineTunnel} from './fake_tunnel';
import {ShadowsocksSessionConfig} from './tunnel';
import {ShadowsocksSessionConfig} from '../model/shadowsocks_session_config';
import {NoOpVpnInstaller, VpnInstaller} from './vpn_installer';

const OUTLINE_PLUGIN_NAME = 'OutlinePlugin';
Expand Down
3 changes: 2 additions & 1 deletion src/www/app/electron_outline_tunnel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@

import * as errors from '../model/errors';

import {Tunnel, TunnelStatus, ShadowsocksSessionConfig} from './tunnel';
import {Tunnel, TunnelStatus} from './tunnel';
import {ShadowsocksSessionConfig} from '../model/shadowsocks_session_config';

export class ElectronOutlineTunnel implements Tunnel {
private statusChangeListener: ((status: TunnelStatus) => void) | null = null;
Expand Down
3 changes: 2 additions & 1 deletion src/www/app/fake_tunnel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@

import * as errors from '../model/errors';

import {Tunnel, TunnelStatus, ShadowsocksSessionConfig} from './tunnel';
import {Tunnel, TunnelStatus} from './tunnel';
import {ShadowsocksSessionConfig} from '../model/shadowsocks_session_config';

// Fake Tunnel implementation for demoing and testing.
// Note that because this implementation does not emit disconnection events, "switching" between
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {SHADOWSOCKS_URI} from 'ShadowsocksConfig';

import * as errors from '../../model/errors';

import {ShadowsocksSessionConfig} from '../tunnel';
import {ShadowsocksSessionConfig} from '../../model/shadowsocks_session_config';

// DON'T use these methods outside of this folder!

Expand All @@ -30,14 +30,15 @@ export function staticKeyToShadowsocksSessionConfig(staticKey: string): Shadowso
method: config.method.data,
password: config.password.data,
prefix: config.extra?.['prefix'],
extra: config.extra,
};
} catch (cause) {
throw new errors.ServerAccessKeyInvalid('Invalid static access key.', {cause});
}
}

function parseShadowsocksSessionConfigJson(maybeJsonText: string): ShadowsocksSessionConfig | null {
const {method, password, server, server_port, prefix} = JSON.parse(maybeJsonText);
const {method, password, server, server_port, prefix, extra} = JSON.parse(maybeJsonText);

// These are the mandatory keys.
const missingKeys = [];
Expand All @@ -58,6 +59,7 @@ function parseShadowsocksSessionConfigJson(maybeJsonText: string): ShadowsocksSe
host: server,
port: server_port,
prefix,
extra,
};
}

Expand Down
6 changes: 4 additions & 2 deletions src/www/app/outline_server_repository/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@
import * as errors from '../../model/errors';
import * as events from '../../model/events';
import {Server, ServerType} from '../../model/server';
import {ShadowsocksSessionConfig} from '../../model/shadowsocks_session_config';

import {Tunnel, TunnelStatus, ShadowsocksSessionConfig} from '../tunnel';
import {Tunnel, TunnelStatus} from '../tunnel';

import {fetchShadowsocksSessionConfig, staticKeyToShadowsocksSessionConfig} from './access_key_serialization';

Expand All @@ -28,7 +29,7 @@ export class OutlineServer implements Server {
private static readonly SUPPORTED_CIPHERS = ['chacha20-ietf-poly1305', 'aes-128-gcm', 'aes-192-gcm', 'aes-256-gcm'];

errorMessageId?: string;
private sessionConfig?: ShadowsocksSessionConfig;
sessionConfig?: ShadowsocksSessionConfig;

constructor(
public readonly id: string,
Expand All @@ -44,6 +45,7 @@ export class OutlineServer implements Server {
break;
case ServerType.STATIC_CONNECTION:
default:
this.accessKey = accessKey;
this.sessionConfig = staticKeyToShadowsocksSessionConfig(accessKey);
break;
}
Expand Down
8 changes: 1 addition & 7 deletions src/www/app/tunnel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

export interface ShadowsocksSessionConfig {
host?: string;
port?: number;
password?: string;
method?: string;
prefix?: string;
}
import {ShadowsocksSessionConfig} from '../model/shadowsocks_session_config';

export const enum TunnelStatus {
CONNECTED,
Expand Down
2 changes: 2 additions & 0 deletions src/www/messages/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@
"server-forgotten-undo": "Server “{serverName}” has been restored.",
"server-rename": "Rename",
"server-rename-complete": "Server renamed",
"server-share": "Share",
"server-share-text": "This is an access key for an Outline Server. To use it, download the Outline app from the App Store or Google Play.",
"servers-menu-item": "Servers",
"servers-page-title": "Outline",
"submit": "Submit",
Expand Down
8 changes: 8 additions & 0 deletions src/www/model/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import {ShadowsocksSessionConfig} from './shadowsocks_session_config';

// TODO: add guidelines for this file

export enum ServerType {
Expand All @@ -31,9 +33,15 @@ export interface Server {
// A type specifying the manner in which the Server connects.
readonly type: ServerType;

// The access key used to connect to the server.
accessKey: string;

// The name of this server, as given by the user.
name: string;

// The configuration used to connect to the server.
sessionConfig?: ShadowsocksSessionConfig;

// The location to pull the session config from on each connection.
sessionConfigLocation?: URL;

Expand Down
22 changes: 22 additions & 0 deletions src/www/model/shadowsocks_session_config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright 2024 The Outline Authors
//
// 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.

export interface ShadowsocksSessionConfig {
host?: string;
port?: number;
password?: string;
method?: string;
prefix?: string;
extra?: {[key: string]: string};
}
1 change: 1 addition & 0 deletions src/www/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
--min-supported-device-width: 320px;

--outline-primary: hsl(170, 60%, 46%);
--outline-warning: hsl(48, 52%, 53%);
--outline-error: hsl(4, 90%, 58%);

--outline-black: hsl(0, 0%, 0%);
Expand Down
4 changes: 2 additions & 2 deletions src/www/views/servers_view/server_list/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ export class ServerList extends PolymerElement {

server-row-card {
margin: 0 auto 8px auto;
height: 130px;
height: 200px;
}

server-hero-card {
height: 400px;
height: 500px;
}
</style>

Expand Down
9 changes: 9 additions & 0 deletions src/www/views/servers_view/server_list_item/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export enum ServerListItemEvent {
DISCONNECT = 'DisconnectPressed',
FORGET = 'ForgetPressed',
RENAME = 'ShowServerRename',
SHARE = 'ShareServer',
}

/**
Expand All @@ -34,6 +35,14 @@ export interface ServerListItem {
id: string;
name: string;
connectionState: ServerConnectionState;
message?: {
type: 'error' | 'warning' | 'info';
content: string;
};
contact?: {
email: string;
};
canShare?: boolean;
}

/**
Expand Down