diff --git a/app/controllers/provider/admin/user/access_tokens_controller.rb b/app/controllers/provider/admin/user/access_tokens_controller.rb index dc2d56ef68..fe248d8487 100644 --- a/app/controllers/provider/admin/user/access_tokens_controller.rb +++ b/app/controllers/provider/admin/user/access_tokens_controller.rb @@ -1,44 +1,59 @@ -class Provider::Admin::User::AccessTokensController < Provider::Admin::User::BaseController - inherit_resources - defaults route_prefix: 'provider_admin_user', resource_class: AccessToken - actions :index, :new, :create, :edit, :update, :destroy - - authorize_resource - activate_menu :account, :personal, :tokens - before_action :authorize_access_tokens - before_action :disable_client_cache - - def create - create! do |success, _failure| - success.html do - flash[:token] = @access_token.id - flash[:notice] = 'Access Token was successfully created.' - redirect_to(collection_url) - end - end - end +# frozen_string_literal: true - def index - index! - @last_access_key = flash[:token] - end +module Provider + module Admin + module User + class AccessTokensController < BaseController + inherit_resources + defaults route_prefix: 'provider_admin_user', resource_class: AccessToken + actions :index, :new, :create, :edit, :update, :destroy - def update - update! do |success, _failure| - success.html do - flash[:notice] = 'Access Token was successfully updated.' - redirect_to(collection_url) - end - end - end + authorize_resource + activate_menu :account, :personal, :tokens + before_action :authorize_access_tokens + before_action :disable_client_cache + before_action :load_presenter, only: %i[new create] - private + def new; end - def authorize_access_tokens - authorize! :manage, :access_tokens, current_user - end + def create + create! do |success, _failure| + success.html do + flash[:token] = @access_token.id + flash[:notice] = 'Access Token was successfully created.' + redirect_to(collection_url) + end + end + end - def begin_of_association_chain - current_user + def index + index! + @last_access_key = flash[:token] + end + + def update + update! do |success, _failure| + success.html do + flash[:notice] = 'Access Token was successfully updated.' + redirect_to(collection_url) + end + end + end + + private + + def load_presenter + @presenter = AccessTokensNewPresenter.new(current_account) + end + + def authorize_access_tokens + authorize! :manage, :access_tokens, current_user + end + + def begin_of_association_chain + current_user + end + end + end end end diff --git a/app/javascript/packs/expiration_date_picker.ts b/app/javascript/packs/expiration_date_picker.ts new file mode 100644 index 0000000000..1425b79051 --- /dev/null +++ b/app/javascript/packs/expiration_date_picker.ts @@ -0,0 +1,22 @@ +import { ExpirationDatePickerWrapper } from 'AccessTokens/components/ExpirationDatePicker' + +import type { Props } from 'AccessTokens/components/ExpirationDatePicker' + +import { safeFromJsonString } from '../src/utilities/json-utils' + +document.addEventListener('DOMContentLoaded', () => { + const containerId = 'expiration-date-picker-container' + const container = document.getElementById(containerId) + + if (!container) { + throw new Error(`Missing container with id "${containerId}"`) + } + + const props = safeFromJsonString(container.dataset.props) + + if (!props) { + throw new Error('Missing props') + } + + ExpirationDatePickerWrapper(props, containerId) +}) diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.scss b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.scss new file mode 100644 index 0000000000..7ca7c4307f --- /dev/null +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.scss @@ -0,0 +1,14 @@ +@import '~@patternfly/patternfly/patternfly-addons.css'; + +.pf-c-calendar-month, .pf-c-form-control-expiration { + width: 50%; +} + +.pf-c-form-control-expiration { + margin-right: 1em; +} + +button.pf-c-form__group-label-help { + min-width: auto; + width: auto; +} diff --git a/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx new file mode 100644 index 0000000000..89a0d154f8 --- /dev/null +++ b/app/javascript/src/AccessTokens/components/ExpirationDatePicker.tsx @@ -0,0 +1,167 @@ +import { useState, useMemo } from 'react' +import { Alert, CalendarMonth, FormGroup, FormSelect, FormSelectOption, Popover } from '@patternfly/react-core' +import OutlinedQuestionCircleIcon from '@patternfly/react-icons/dist/js/icons/outlined-question-circle-icon' + +import { createReactWrapper } from 'utilities/createReactWrapper' + +import type { FunctionComponent, FormEvent } from 'react' + +import './ExpirationDatePicker.scss' + +interface ExpirationItem { + id: string; + label: string; + period: number; // In seconds +} + +const collection: ExpirationItem[] = [ + { id: '7', label: '7 days', period: 7 }, + { id: '30', label: '30 days', period: 30 }, + { id: '60', label: '60 days', period: 60 }, + { id: '90', label: '90 days', period: 90 }, + { id: 'custom', label: 'Custom...', period: 0 }, + { id: 'no-exp', label: 'No expiration', period: 0 } +] + +const dayMs = 60 * 60 * 24 * 1000 + +interface Props { + id: string; + label: string | null; + tzOffset?: number; +} + +const ExpirationDatePicker: FunctionComponent = ({ id, label, tzOffset }) => { + const [selectedItem, setSelectedItem] = useState(collection[0]) + const [pickedDate, setPickedDate] = useState(new Date()) + const fieldName = `human_${id}` + const fieldLabel = label ?? 'Expires in' + + const fieldDate = useMemo(() => { + if (selectedItem.period === 0) return null + + return new Date(new Date().getTime() + selectedItem.period * dayMs) + }, [selectedItem]) + + const dateValue = useMemo(() => { + let value = '' + + if (fieldDate) { + value = fieldDate.toISOString() + } else if (selectedItem.id === 'custom' ) { + value = pickedDate.toISOString() + } + + return value + }, [fieldDate, selectedItem, pickedDate]) + + const formattedDateValue = useMemo(() => { + if (!dateValue) return + + const formatter = Intl.DateTimeFormat('en-US', { + month: 'long', day: 'numeric', year: 'numeric', hour: 'numeric', minute: 'numeric', hour12: false + }) + const date = new Date(dateValue) + + return formatter.format(date) + }, [dateValue]) + + const fieldHint = useMemo(() => { + if (!formattedDateValue) return + + return `The token will expire on ${formattedDateValue}` + }, [formattedDateValue]) + + const tzMismatch = useMemo(() => { + if (tzOffset === undefined) return + + // Timezone offset in the same format as ActiveSupport + const jsTzOffset = new Date().getTimezoneOffset() * -60 + + return jsTzOffset !== tzOffset + }, [tzOffset]) + + const labelIcon = useMemo(() => { + if (!tzMismatch) return + + return ( + + Your local time zone differs from the provider default. + The token will expire at the time you selected in your local time zone. +

+ )} + headerContent={( + Time zone mismatch + )} + > + + + ) + }, [tzMismatch]) + + const handleOnChange = (_value: string, event: FormEvent) => { + const value = (event.target as HTMLSelectElement).value + const selected = collection.find(i => i.id === value) ?? null + + if (selected === null) return + + setSelectedItem(selected) + setPickedDate(new Date()) + } + + return ( + <> + + + {collection.map((item: ExpirationItem) => { + return ( + + ) + })} + + + + {selectedItem.id === 'custom' && ( + + )} + {dateValue === '' && ( + <> +
+ + It is strongly recommended that you set an expiration date for your token to help keep your information + secure + + + )} + + ) +} + +const ExpirationDatePickerWrapper = (props: Props, containerId: string): void => { createReactWrapper(, containerId) } + +export type { ExpirationItem, Props } +export { ExpirationDatePicker, ExpirationDatePickerWrapper } diff --git a/app/models/access_token.rb b/app/models/access_token.rb index 1b1b79cad6..c38d0f224d 100644 --- a/app/models/access_token.rb +++ b/app/models/access_token.rb @@ -140,7 +140,7 @@ def validate_scope_exists def expires_at=(value) return if value.blank? - DateTime.strptime(value) + DateTime.parse(value) super value rescue StandardError diff --git a/app/presenters/provider/admin/user/access_tokens_new_presenter.rb b/app/presenters/provider/admin/user/access_tokens_new_presenter.rb new file mode 100644 index 0000000000..ac85c33f5f --- /dev/null +++ b/app/presenters/provider/admin/user/access_tokens_new_presenter.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class Provider::Admin::User::AccessTokensNewPresenter + + def initialize(provider) + @provider = provider + @timezone = ActiveSupport::TimeZone.new(provider.timezone) + end + + def provider_timezone_offset + @timezone.utc_offset + end +end diff --git a/app/views/provider/admin/user/access_tokens/_form.html.slim b/app/views/provider/admin/user/access_tokens/_form.html.slim index eccdf3a684..8a32a15976 100644 --- a/app/views/provider/admin/user/access_tokens/_form.html.slim +++ b/app/views/provider/admin/user/access_tokens/_form.html.slim @@ -7,3 +7,14 @@ = form.input :permission, as: :patternfly_select, collection: @access_token.available_permissions, include_blank: false + +- if @access_token.persisted? + .pf-c-form__group + .pf-c-form__group-label + label.pf-c-form__label + span.pf-c-form__label-text + = t('access_token_options.expires_at') + .pf-c-form__group-control + = @access_token.expires_at.present? ? l(@access_token.expires_at) : t('access_token_options.no_expiration') +- else + div id='expiration-date-picker-container' data-props={ id: 'access_token[expires_at]', label: t('access_token_options.expires_in'), tzOffset: @presenter.provider_timezone_offset }.to_json diff --git a/app/views/provider/admin/user/access_tokens/index.html.slim b/app/views/provider/admin/user/access_tokens/index.html.slim index d571f646db..163a7edcc6 100644 --- a/app/views/provider/admin/user/access_tokens/index.html.slim +++ b/app/views/provider/admin/user/access_tokens/index.html.slim @@ -31,6 +31,13 @@ dd class="pf-c-description-list__description" div class="pf-c-description-list__text" = token.human_permission + div class="pf-c-description-list__group" + dt class="pf-c-description-list__term" + span class="pf-c-description-list__text" + | Expires at + dd class="pf-c-description-list__description" + div class="pf-c-description-list__text" + = token.expires_at.present? ? l(token.expires_at) : t('access_token_options.no_expiration') div class="pf-c-description-list__group" dt class="pf-c-description-list__term" span class="pf-c-description-list__text" @@ -58,6 +65,7 @@ tr role="row" th role="columnheader" scope="col" Name th role="columnheader" scope="col" Scopes + th role="columnheader" scope="col" Expiration th role="columnheader" scope="col" Permission th role="columnheader" scope="col" class="pf-c-table__action pf-m-fit-content" = fancy_link_to 'Add Access Token', new_provider_admin_user_access_token_path, class: 'new' if allowed_scopes.any? @@ -67,6 +75,7 @@ tr role="row" td role="cell" data-label="Name" = token.name td role="cell" data-label="Scopes" = token.human_scopes.to_sentence + td role="cell" data-label="Expiration" = token.expires_at.present? ? l(token.expires_at) : t('access_token_options.no_expiration') td role="cell" data-label="Permission" = token.human_permission td role="cell" class="pf-c-table__action" div class="pf-c-overflow-menu" diff --git a/app/views/provider/admin/user/access_tokens/new.html.slim b/app/views/provider/admin/user/access_tokens/new.html.slim index be3ace8aaa..d69e35a522 100644 --- a/app/views/provider/admin/user/access_tokens/new.html.slim +++ b/app/views/provider/admin/user/access_tokens/new.html.slim @@ -2,6 +2,7 @@ - content_for :javascripts do = javascript_packs_with_chunks_tag 'pf_form' + = javascript_packs_with_chunks_tag 'expiration_date_picker' div class="pf-c-card" div class="pf-c-card__body" diff --git a/config/locales/en.yml b/config/locales/en.yml index 3c98638064..966ee00e2e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -65,6 +65,9 @@ en: cms: 'Developer Portal API' ro: 'Read Only' rw: 'Read & Write' + no_expiration: 'Never expires' + expires_at: 'Expires at' + expires_in: 'Expires in' notification_category_titles: account: 'Accounts' application: 'Applications' diff --git a/features/provider/admin/user/access_tokens.feature b/features/provider/admin/user/access_tokens.feature index 191277e5f2..eb9222edb0 100644 --- a/features/provider/admin/user/access_tokens.feature +++ b/features/provider/admin/user/access_tokens.feature @@ -24,9 +24,9 @@ Feature: Provider Admin Access tokens Scenario: Tokens are listed in a table Then the table should contain the following: - | Name | Scopes | Permission | - | Potato | Analytics API | Read Only | - | Banana | Billing API | Read & Write | + | Name | Scopes | Expiration | Permission | + | Potato | Analytics API | Never expires | Read Only | + | Banana | Billing API | Never expires | Read & Write | Rule: New page Background: @@ -42,6 +42,7 @@ Feature: Provider Admin Access tokens Then there is a required field "Name" And there is a required field "Scopes" And there is a required field "Permission" + And there is a required field "Expires in" And the submit button is enabled Scenario: Create access tokens without required fields @@ -49,26 +50,29 @@ Feature: Provider Admin Access tokens Then field "Name" has inline error "can't be blank" And field "Scopes" has inline error "select at least one scope" And field "Permission" has no inline error + And field "Expires in" has no inline error Scenario: Create access token When they press "Create Access Token" And the form is submitted with: - | Name | LeToken | - | Analytics API | Yes | - | Permission | Read & Write | + | Name | LeToken | + | Analytics API | Yes | + | Permission | Read & Write | + | Expires in | No expiration | Then the current page is the personal tokens page And they should see the flash message "Access token was successfully created" And should see the following details: - | Name | LeToken | - | Scopes | Analytics API | - | Permission | Read & Write | + | Name | LeToken | + | Scopes | Analytics API | + | Permission | Read & Write | + | Expires at | Never expires | And there should be a link to "I have copied the token" Rule: Edit page Background: Given the provider has the following access tokens: - | Name | Scopes | Permission | - | LeToken | Billing API, Analytics API | Read Only | + | Name | Scopes | Permission | Expires at | + | LeToken | Billing API, Analytics API | Read Only | 2030-01-01T00:00:00Z | And they go to the access token's edit page Scenario: Navigation to edit page diff --git a/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx b/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx new file mode 100644 index 0000000000..849d5fca2f --- /dev/null +++ b/spec/javascripts/AccessTokens/components/ExpirationDatePicker.spec.tsx @@ -0,0 +1,162 @@ +import { mount } from 'enzyme' + +import { ExpirationDatePicker } from 'AccessTokens/components/ExpirationDatePicker' + +import type { ExpirationItem, Props } from 'AccessTokens/components/ExpirationDatePicker' +import type { ReactWrapper } from 'enzyme' + +const msExp = /\.\d{3}Z$/ + +const defaultProps: Props = { + id: 'expires_at', + label: 'Expires in', + tzOffset: 0 +} + +const mountWrapper = (props: Partial = {}) => mount() + +const selectItem = (wrapper: ReactWrapper>, item: ExpirationItem) => { + wrapper.find('select.pf-c-form-control-expiration').simulate('change', { target: { value: item.id.toString() } }) +} + +const pickDate = (wrapper: ReactWrapper>) => { + /* + * Pick tomorrow, to do so, we get the date selected by default which is today and click the next one. + * It could happen that today is the last day in the calendar, in that case we pick the previous day, yesterday. + * In any case, we return the picked date to the caller. + */ + const targetDate = new Date() + targetDate.setHours(0) + targetDate.setMinutes(0) + targetDate.setSeconds(0) + targetDate.setMilliseconds(0) + + const tomorrowButton = wrapper.find('.pf-m-selected + td > button') + + if (tomorrowButton.length === 0) { + // No tomorrow, pick yesterday + const dayButtons = wrapper.find('button.pf-c-calendar-month__date') + const yesterdayButton = dayButtons.at(dayButtons.length - 2) + + yesterdayButton.simulate('click') + targetDate.setDate(targetDate.getDate() - 1) + return targetDate + } + + tomorrowButton.simulate('click') + targetDate.setDate(targetDate.getDate() + 1) + return targetDate +} + +const dateFormatter = Intl.DateTimeFormat('en-US', { + month: 'long', day: 'numeric', year: 'numeric', hour: 'numeric', minute: 'numeric', hour12: false +}) + +it('should render itself', () => { + const wrapper = mountWrapper() + expect(wrapper.exists()).toEqual(true) +}) + +describe('select a period', () => { + const targetItem: ExpirationItem = { id: '90', label: '90 days', period: 90 } + + it('should update hint to the correct date', () => { + const wrapper = mountWrapper() + const targetDate = new Date(new Date().getTime() + 1000 * 60 * 60 * 24 * targetItem.period) + const expectedHint = `The token will expire on ${dateFormatter.format(targetDate)}` + + selectItem(wrapper, targetItem) + const hint = wrapper.find('.pf-c-form__helper-text').text() + + expect(hint).toBe(expectedHint) + }) + + it('should update hidden input value to the correct timestamp', () => { + const wrapper = mountWrapper() + const targetDate = new Date(new Date().getTime() + 1000 * 60 * 60 * 24 * targetItem.period) + const expectedValue = targetDate.toISOString().replace(msExp, 'Z') + + selectItem(wrapper, targetItem) + const value = (wrapper.find(`input#${defaultProps.id}`).prop('value') as string).replace(msExp, 'Z') + + expect(value).toBe(expectedValue) + }) +}) + +describe('select "Custom"', () => { + const targetItem: ExpirationItem = { id: 'custom', label: 'Custom...', period: 0 } + + it('should show a calendar', () => { + const wrapper = mountWrapper() + + selectItem(wrapper, targetItem) + const calendar = wrapper.find('.pf-c-calendar-month') + + expect(calendar.exists()).toBe(true) + }) + + describe('pick a date from the calendar', () => { + it('should update hint to the correct date', () => { + const wrapper = mountWrapper() + + selectItem(wrapper, targetItem) + const targetDate = pickDate(wrapper) + const expectedHint = `The token will expire on ${dateFormatter.format(targetDate)}` + const hint = wrapper.find('.pf-c-form__helper-text').text() + + expect(hint).toBe(expectedHint) + }) + + it('should update hidden input value to the correct timestamp', () => { + const wrapper = mountWrapper() + + selectItem(wrapper, targetItem) + const targetDate = pickDate(wrapper) + const expectedValue = targetDate.toISOString().replace(msExp, 'Z') + const value = (wrapper.find(`input#${defaultProps.id}`).prop('value') as string).replace(msExp, 'Z') + + expect(value).toBe(expectedValue) + }) + }) +}) + +describe('select "No expiration"', () => { + const targetItem: ExpirationItem = { id: 'no-exp', label: 'No expiration', period: 0 } + + it('should show a warning', () => { + const wrapper = mountWrapper() + + selectItem(wrapper, targetItem) + const warning = wrapper.find('.pf-c-alert.pf-m-warning') + + expect(warning.exists()).toBe(true) + }) + + it('should update hidden input value to empty', () => { + const wrapper = mountWrapper() + const expectedValue = '' + + selectItem(wrapper, targetItem) + const value = wrapper.find(`input#${defaultProps.id}`).prop('value') + + expect(value).toBe(expectedValue) + }) +}) + +describe('time zone matches', () => { + it('should not show a warning', ()=> { + jest.spyOn(Date.prototype, 'getTimezoneOffset').mockImplementation(() => (0)) + const wrapper = mountWrapper() + + expect(wrapper.exists('.pf-c-form__group-label-help')).toEqual(false) + }) +}) + +describe('time zone mismatches', () => { + it('should show a warning', ()=> { + jest.spyOn(Date.prototype, 'getTimezoneOffset').mockImplementation(() => (-120)) + const wrapper = mountWrapper() + + expect(wrapper.exists('.pf-c-form__group-label-help')).toEqual(true) + }) +}) \ No newline at end of file