diff --git a/src/api/apiTypes.js b/src/api/apiTypes.js index ac7ef381ca8..f79c2208be5 100644 --- a/src/api/apiTypes.js +++ b/src/api/apiTypes.js @@ -5,7 +5,4 @@ export type * from './modelTypes'; export type * from './eventTypes'; export type * from './initialDataTypes'; -export type { - AuthenticationMethods, - ApiResponseServerSettings, -} from './settings/getServerSettings'; +export type * from './settings/getServerSettings'; diff --git a/src/api/settings/getServerSettings.js b/src/api/settings/getServerSettings.js index af7a94df294..2d0afe23837 100644 --- a/src/api/settings/getServerSettings.js +++ b/src/api/settings/getServerSettings.js @@ -14,9 +14,19 @@ export type AuthenticationMethods = { ... }; +export type ExternalAuthenticationMethod = {| + name: string, + display_name: string, + display_icon: string | null, + login_url: string, + signup_url: string, +|}; + export type ApiResponseServerSettings = {| ...ApiResponseSuccess, authentication_methods: AuthenticationMethods, + // external_authentication_methods added for server v2.1 + external_authentication_methods?: ExternalAuthenticationMethod[], email_auth_enabled: boolean, push_notifications_enabled: boolean, realm_description: string, diff --git a/src/start/AuthScreen.js b/src/start/AuthScreen.js index 58b34e9497a..d1a732fadca 100644 --- a/src/start/AuthScreen.js +++ b/src/start/AuthScreen.js @@ -4,7 +4,12 @@ import React, { PureComponent } from 'react'; import { Linking } from 'react-native'; import type { NavigationScreenProp } from 'react-navigation'; -import type { AuthenticationMethods, Dispatch, ApiResponseServerSettings } from '../types'; +import type { + AuthenticationMethods, + Dispatch, + ExternalAuthenticationMethod, + ApiResponseServerSettings, +} from '../types'; import { IconPrivate, IconGoogle, IconGitHub, IconWindows, IconTerminal } from '../common/Icons'; import type { IconType } from '../common/Icons'; import { connect } from '../react-redux'; @@ -34,7 +39,8 @@ type AuthenticationMethodDetails = {| action: 'dev' | 'password' | {| url: string |}, |}; -const authentications: AuthenticationMethodDetails[] = [ +// Methods that don't show up in external_authentication_methods. +const availableDirectMethods: AuthenticationMethodDetails[] = [ { name: 'dev', displayName: 'dev account', @@ -53,6 +59,19 @@ const authentications: AuthenticationMethodDetails[] = [ Icon: IconPrivate, action: 'password', }, + { + // This one might move to external_authentication_methods in the future. + name: 'remoteuser', + displayName: 'SSO', + Icon: IconPrivate, + action: { url: 'accounts/login/sso/' }, + }, +]; + +// Methods that are covered in external_authentication_methods by servers +// which have that key (Zulip Server v2.1+). We refer to this array for +// servers that don't. +const availableExternalMethods: AuthenticationMethodDetails[] = [ { name: 'google', displayName: 'Google', @@ -74,20 +93,22 @@ const authentications: AuthenticationMethodDetails[] = [ Icon: IconWindows, action: { url: '/accounts/login/social/azuread-oauth2' }, }, - { - name: 'remoteuser', - displayName: 'SSO', - Icon: IconPrivate, - action: { url: 'accounts/login/sso/' }, - }, ]; +const externalMethodIcons = new Map([ + ['google', IconGoogle], + ['github', IconGitHub], + ['azuread', IconWindows], +]); + /** Exported for tests only. */ export const activeAuthentications = ( authenticationMethods: AuthenticationMethods, + externalAuthenticationMethods: ExternalAuthenticationMethod[] | void, ): AuthenticationMethodDetails[] => { const result = []; - authentications.forEach(auth => { + + availableDirectMethods.forEach(auth => { if (!authenticationMethods[auth.name]) { return; } @@ -98,6 +119,37 @@ export const activeAuthentications = ( } result.push(auth); }); + + if (!externalAuthenticationMethods) { + // Server doesn't speak new API; get these methods from the old one. + availableExternalMethods.forEach(auth => { + if (authenticationMethods[auth.name]) { + result.push(auth); + } + }); + } else { + // We have info from new API; ignore old one for these methods. + externalAuthenticationMethods.forEach(method => { + if (result.some(({ name }) => name === method.name)) { + // Ignore duplicate. + return; + } + + // The server provides icons as image URLs; but we have our own built + // in, which we don't have to load and can color to match the button. + // TODO perhaps switch to server's, for the sake of SAML where ours is + // generic and the server may have a more specific one. + const Icon = externalMethodIcons.get(method.name) ?? IconPrivate; + + result.push({ + name: method.name, + displayName: method.display_name, + Icon, + action: { url: method.login_url }, + }); + }); + } + return result; }; @@ -130,8 +182,10 @@ class AuthScreen extends PureComponent { } }); + const { serverSettings } = this.props.navigation.state.params; const authList = activeAuthentications( - this.props.navigation.state.params.serverSettings.authentication_methods, + serverSettings.authentication_methods, + serverSettings.external_authentication_methods, ); if (authList.length === 1) { this.handleAuth(authList[0]); @@ -187,7 +241,10 @@ class AuthScreen extends PureComponent { name={serverSettings.realm_name} iconUrl={getFullUrl(serverSettings.realm_icon, this.props.realm)} /> - {activeAuthentications(serverSettings.authentication_methods).map(auth => ( + {activeAuthentications( + serverSettings.authentication_methods, + serverSettings.external_authentication_methods, + ).map(auth => ( { +describe('activeAuthentications: external_authentication_methods (server v2.1+ API)', () => { + test('clobbers hardcoded info for same methods', () => { + expect( + activeAuthentications({ google: true }, [ + { + name: 'google', + signup_url: '/accounts/register/social/google', + display_icon: '/static/images/landing-page/logos/googl_e-icon.png', + display_name: 'Google', + login_url: '/accounts/login/social/google', + }, + ]), + ).toMatchObject([ + { + name: 'google', + displayName: 'Google', + // NB different from hardcoded URL for same method + action: { url: '/accounts/login/social/google' }, + }, + ]); + }); + + test('supplements internal methods', () => { + expect(activeAuthentications({ password: true, google: true }, [])).toMatchObject([ + { name: 'password' }, + ]); + }); + + test('handles example SAML data', () => { + expect( + activeAuthentications({ saml: true }, [ + { + name: 'saml:okta', + display_name: 'SAML', + display_icon: null, + login_url: '/accounts/login/social/saml/okta', + signup_url: '/accounts/register/social/saml/okta', + }, + ]), + ).toMatchObject([ + { + name: 'saml:okta', + displayName: 'SAML', + action: { url: '/accounts/login/social/saml/okta' }, + }, + ]); + }); +}); + +describe('activeAuthentications: old server API', () => { test('empty auth methods object result in no available authentications', () => { const authenticationMethods = {}; - const actual = activeAuthentications(authenticationMethods); + const actual = activeAuthentications(authenticationMethods, undefined); expect(actual).toEqual([]); }); @@ -17,7 +66,7 @@ describe('activeAuthentications', () => { password: true, }; - const actual = activeAuthentications(authenticationMethods); + const actual = activeAuthentications(authenticationMethods, undefined); expect(actual).toHaveLength(2); }); @@ -28,7 +77,7 @@ describe('activeAuthentications', () => { password: true, }; - const actual = activeAuthentications(authenticationMethods); + const actual = activeAuthentications(authenticationMethods, undefined); expect(actual).toHaveLength(1); }); @@ -44,7 +93,7 @@ describe('activeAuthentications', () => { remoteuser: true, }; - const actual = activeAuthentications(authenticationMethods); + const actual = activeAuthentications(authenticationMethods, undefined); expect(actual).toHaveLength(6); }); @@ -55,7 +104,7 @@ describe('activeAuthentications', () => { unknown: true, }; - const actual = activeAuthentications(authenticationMethods); + const actual = activeAuthentications(authenticationMethods, undefined); expect(actual).toHaveLength(1); });