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

Jump to bottom

Web app online config #921

Closed
wants to merge 3 commits into from
Closed

Web app online config #921

wants to merge 3 commits into from

Conversation

alalamav
Copy link
Contributor

@alalamav alalamav commented Dec 9, 2020 •
edited
Loading

Implements SIP008 online config in the web app per #891.

  • Refactors ServerConfig to include the proxy configuration source. Performs a data migration on local storage.
  • The Tunnel interface exposes a method, fetchProxyConfig, to retrieve the proxy configuration.
    • The ProxyConfigResponse type enables the web app to update the configuration source on redirects and surface HTTP status codes.
  • OutlineServer retrieves the proxy configuration upon connecting.
  • The server card UI displays the proxy configuration source for online config servers while disconnected and the proxy configuration host while connected.

TODO

  • Define and localize error messages for fetch proxy config failures.
  • Implement a policy to select a proxy configuration out of many.
  • Decide whether to support ssconf:// in addtion to https://.
  • Decide a default name for online config servers.

@alalamav alalamav self-assigned this Dec 9, 2020
fortuna
fortuna requested changes Feb 4, 2021
View reviewed changes
Copy link
Collaborator

@fortuna fortuna left a comment

Choose a reason for hiding this comment

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

I'm glad to see this change!

src/www/app/outline_server.ts
}

async connect() {
try {
if (this.config.source) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's move this logic to get the config to a ConfigSource object, with a getConfigForNewConnection().
This will separate the concerns. The OutlineServer does not have to know how the config is obtained.

src/www/model/server.ts
@@ -40,10 +43,21 @@ export interface Server {
checkReachable(): Promise<boolean>;
}

export type ServerConfig = ShadowsocksConfig;

export interface ServerConfig {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's remove ServerConfig from the model. That's a persistency/application concept.
For persistency, let's use {name, access_key}, This way we reuse the existing serialization format. No need to have two different formats.

src/www/model/server.ts

export interface ServerRepository {
add(serverConfig: ServerConfig): void;
update(serverId: string, newConfig: ServerConfig): void;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This could be a method on the ConfigSource instead. On permanent redirection + successful connection, we can call configSource.setAccessKey(newAccessKey), and the implementation takes care of persisting that.

src/www/model/shadowsocks.ts
@@ -20,3 +20,8 @@ export interface ShadowsocksConfig {
method?: string;
name?: string;
}

// Endpoint to retrieve Shadowsocks proxy configurations.
export interface ShadowsocksConfigSource {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The ConfigSource doesn't need to live in the model. It can be defined in the repository module.

src/www/ui_components/server-card.html
@@ -155,7 +159,7 @@
></server-connection-viz>
<div id="serverInfo">
<div id="serverName">[[serverName]]</div>
<div id="serverHost">[[serverHost]]:[[serverPort]]</div>
<div id="serverHost">[[serverHost]]</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps we should still show server and port, but only while connected?
This is really a UI question.

/cc @cjhenck

src/www/app/persistent_server.ts
const serversJsonV1 = JSON.stringify(configByIdV1);
storage.setItem(PersistentServerRepository.SERVERS_STORAGE_KEY, serversJsonV1);
}

function configsMatch(left: ServerConfig, right: ServerConfig) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we use the access key, this can be a simple ==

src/www/app/persistent_server.ts
@@ -35,13 +38,20 @@ export type PersistentServerFactory =
// Maintains a persisted set of servers and liaises with the core.
export class PersistentServerRepository implements ServerRepository {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can move this code to the outline_server.ts file.

src/www/app/persistent_server.ts
@@ -58,15 +68,25 @@ export class PersistentServerRepository implements ServerRepository {
if (alreadyAddedServer) {
throw new ServerAlreadyAdded(alreadyAddedServer);
}
if (!OutlineServer.isServerCipherSupported(serverConfig.method)) {
throw new ShadowsocksUnsupportedCipher(serverConfig.method || 'unknown');
if (!isServerCipherSupported(serverConfig)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need to fetch the config from the ConfigSource here, so we can validate it and make sure it works.

fortuna
fortuna reviewed Feb 4, 2021
View reviewed changes
src/types/ambient/outlinePlugin.d.ts

config: ShadowsocksConfig;
config?: ShadowsocksConfig;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this shouldn't be explosed?

src/types/ambient/outlinePlugin.d.ts

readonly id: string;

fetchProxyConfig(source: ShadowsocksConfigSource): Promise<ProxyConfigResponse>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This would go away, since the TS code would already be giving you the Shadowsocks config to use.

@daniellacosse daniellacosse linked an issue Dec 20, 2021 that may be closed by this pull request
Basic support for Online Config #891
Closed
@sbruens sbruens closed this Mar 5, 2024
@sbruens sbruens deleted the alalama-online-config-www branch March 5, 2024 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fortuna fortuna fortuna requested changes

Assignees

@alalamav alalamav

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Basic support for Online Config
3 participants
@alalamav @fortuna @sbruens