diff --git a/app/assets/javascripts/admin/adapters/api-key.js.es6 b/app/assets/javascripts/admin/adapters/api-key.js.es6
new file mode 100644
index 0000000000..a473f66e08
--- /dev/null
+++ b/app/assets/javascripts/admin/adapters/api-key.js.es6
@@ -0,0 +1,11 @@
+import RESTAdapter from "discourse/adapters/rest";
+
+export default RESTAdapter.extend({
+ basePath() {
+ return "/admin/api/";
+ },
+
+ apiNameFor() {
+ return "key";
+ }
+});
diff --git a/app/assets/javascripts/admin/controllers/admin-api-keys-index.js.es6 b/app/assets/javascripts/admin/controllers/admin-api-keys-index.js.es6
new file mode 100644
index 0000000000..b087269626
--- /dev/null
+++ b/app/assets/javascripts/admin/controllers/admin-api-keys-index.js.es6
@@ -0,0 +1,13 @@
+import { popupAjaxError } from "discourse/lib/ajax-error";
+
+export default Ember.Controller.extend({
+ actions: {
+ revokeKey(key) {
+ key.revoke().catch(popupAjaxError);
+ },
+
+ undoRevokeKey(key) {
+ key.undoRevoke().catch(popupAjaxError);
+ }
+ }
+});
diff --git a/app/assets/javascripts/admin/controllers/admin-api-keys-new.js.es6 b/app/assets/javascripts/admin/controllers/admin-api-keys-new.js.es6
new file mode 100644
index 0000000000..f4d56c0a05
--- /dev/null
+++ b/app/assets/javascripts/admin/controllers/admin-api-keys-new.js.es6
@@ -0,0 +1,39 @@
+import { default as computed } from "ember-addons/ember-computed-decorators";
+import { popupAjaxError } from "discourse/lib/ajax-error";
+
+export default Ember.Controller.extend({
+ userModes: [
+ { id: "all", name: I18n.t("admin.api.all_users") },
+ { id: "single", name: I18n.t("admin.api.single_user") }
+ ],
+
+ @computed("userMode")
+ showUserSelector(mode) {
+ return mode === "single";
+ },
+
+ @computed("model.description", "model.username", "userMode")
+ saveDisabled(description, username, userMode) {
+ if (Ember.isBlank(description)) return true;
+ if (userMode === "single" && Ember.isBlank(username)) return true;
+ return false;
+ },
+
+ actions: {
+ changeUserMode(value) {
+ if (value === "all") {
+ this.model.set("username", null);
+ }
+ this.set("userMode", value);
+ },
+
+ save() {
+ this.model
+ .save()
+ .then(() => {
+ this.transitionToRoute("adminApiKeys.show", this.model.id);
+ })
+ .catch(popupAjaxError);
+ }
+ }
+});
diff --git a/app/assets/javascripts/admin/controllers/admin-api-keys-show.js.es6 b/app/assets/javascripts/admin/controllers/admin-api-keys-show.js.es6
new file mode 100644
index 0000000000..2e19ea5779
--- /dev/null
+++ b/app/assets/javascripts/admin/controllers/admin-api-keys-show.js.es6
@@ -0,0 +1,54 @@
+import { bufferedProperty } from "discourse/mixins/buffered-content";
+import { popupAjaxError } from "discourse/lib/ajax-error";
+import { empty } from "@ember/object/computed";
+
+export default Ember.Controller.extend(bufferedProperty("model"), {
+ isNew: empty("model.id"),
+
+ actions: {
+ saveDescription() {
+ const buffered = this.buffered;
+ const attrs = buffered.getProperties("description");
+
+ this.model
+ .save(attrs)
+ .then(() => {
+ this.set("editingDescription", false);
+ this.rollbackBuffer();
+ })
+ .catch(popupAjaxError);
+ },
+
+ cancel() {
+ const id = this.get("userField.id");
+ if (Ember.isEmpty(id)) {
+ this.destroyAction(this.userField);
+ } else {
+ this.rollbackBuffer();
+ this.set("editing", false);
+ }
+ },
+
+ editDescription() {
+ this.toggleProperty("editingDescription");
+ if (!this.editingDescription) {
+ this.rollbackBuffer();
+ }
+ },
+
+ revokeKey(key) {
+ key.revoke().catch(popupAjaxError);
+ },
+
+ deleteKey(key) {
+ key
+ .destroyRecord()
+ .then(() => this.transitionToRoute("adminApiKeys.index"))
+ .catch(popupAjaxError);
+ },
+
+ undoRevokeKey(key) {
+ key.undoRevoke().catch(popupAjaxError);
+ }
+ }
+});
diff --git a/app/assets/javascripts/admin/controllers/admin-api-keys.js.es6 b/app/assets/javascripts/admin/controllers/admin-api-keys.js.es6
index 9cd1682ff6..e69de29bb2 100644
--- a/app/assets/javascripts/admin/controllers/admin-api-keys.js.es6
+++ b/app/assets/javascripts/admin/controllers/admin-api-keys.js.es6
@@ -1,42 +0,0 @@
-import ApiKey from "admin/models/api-key";
-import { default as computed } from "ember-addons/ember-computed-decorators";
-import Controller from "@ember/controller";
-
-export default Controller.extend({
- @computed("model.[]")
- hasMasterKey(model) {
- return !!model.findBy("user", null);
- },
-
- actions: {
- generateMasterKey() {
- ApiKey.generateMasterKey().then(key => this.model.pushObject(key));
- },
-
- regenerateKey(key) {
- bootbox.confirm(
- I18n.t("admin.api.confirm_regen"),
- I18n.t("no_value"),
- I18n.t("yes_value"),
- result => {
- if (result) {
- key.regenerate();
- }
- }
- );
- },
-
- revokeKey(key) {
- bootbox.confirm(
- I18n.t("admin.api.confirm_revoke"),
- I18n.t("no_value"),
- I18n.t("yes_value"),
- result => {
- if (result) {
- key.revoke().then(() => this.model.removeObject(key));
- }
- }
- );
- }
- }
-});
diff --git a/app/assets/javascripts/admin/controllers/admin-user-index.js.es6 b/app/assets/javascripts/admin/controllers/admin-user-index.js.es6
index 9a25f679ef..47a54b8ebf 100644
--- a/app/assets/javascripts/admin/controllers/admin-user-index.js.es6
+++ b/app/assets/javascripts/admin/controllers/admin-user-index.js.es6
@@ -258,10 +258,6 @@ export default Controller.extend(CanCheckEmails, {
.finally(() => this.toggleProperty("editingTitle"));
},
- generateApiKey() {
- this.model.generateApiKey();
- },
-
saveCustomGroups() {
const currentIds = this.customGroupIds;
const bufferedIds = this.customGroupIdsBuffer;
@@ -294,32 +290,6 @@ export default Controller.extend(CanCheckEmails, {
resetPrimaryGroup() {
this.set("model.primary_group_id", this.originalPrimaryGroupId);
- },
-
- regenerateApiKey() {
- bootbox.confirm(
- I18n.t("admin.api.confirm_regen"),
- I18n.t("no_value"),
- I18n.t("yes_value"),
- result => {
- if (result) {
- this.model.generateApiKey();
- }
- }
- );
- },
-
- revokeApiKey() {
- bootbox.confirm(
- I18n.t("admin.api.confirm_revoke"),
- I18n.t("no_value"),
- I18n.t("yes_value"),
- result => {
- if (result) {
- this.model.revokeApiKey();
- }
- }
- );
}
}
});
diff --git a/app/assets/javascripts/admin/models/admin-user.js.es6 b/app/assets/javascripts/admin/models/admin-user.js.es6
index 891a2923c6..e99c84e0b4 100644
--- a/app/assets/javascripts/admin/models/admin-user.js.es6
+++ b/app/assets/javascripts/admin/models/admin-user.js.es6
@@ -4,7 +4,6 @@ import { ajax } from "discourse/lib/ajax";
import computed from "ember-addons/ember-computed-decorators";
import { propertyNotEqual } from "discourse/lib/computed";
import { popupAjaxError } from "discourse/lib/ajax-error";
-import ApiKey from "admin/models/api-key";
import Group from "discourse/models/group";
import { userPath } from "discourse/lib/url";
@@ -57,16 +56,6 @@ const AdminUser = Discourse.User.extend({
);
},
- generateApiKey() {
- return ajax(`/admin/users/${this.id}/generate_api_key`, {
- type: "POST"
- }).then(result => {
- const apiKey = ApiKey.create(result.api_key);
- this.set("api_key", apiKey);
- return apiKey;
- });
- },
-
groupAdded(added) {
return ajax(`/admin/users/${this.id}/groups`, {
type: "POST",
diff --git a/app/assets/javascripts/admin/models/api-key.js.es6 b/app/assets/javascripts/admin/models/api-key.js.es6
index 62a4e003bf..95d8e1914c 100644
--- a/app/assets/javascripts/admin/models/api-key.js.es6
+++ b/app/assets/javascripts/admin/models/api-key.js.es6
@@ -1,47 +1,55 @@
import AdminUser from "admin/models/admin-user";
+import RestModel from "discourse/models/rest";
import { ajax } from "discourse/lib/ajax";
+import computed from "ember-addons/ember-computed-decorators";
-const KEY_ENDPOINT = "/admin/api/key";
-const KEYS_ENDPOINT = "/admin/api/keys";
+const ApiKey = RestModel.extend({
+ user: Ember.computed("_user", {
+ get() {
+ return this._user;
+ },
+ set(key, value) {
+ if (value && !(value instanceof AdminUser)) {
+ this.set("_user", AdminUser.create(value));
+ } else {
+ this.set("_user", value);
+ }
+ return this._user;
+ }
+ }),
-const ApiKey = Discourse.Model.extend({
- regenerate() {
- return ajax(KEY_ENDPOINT, {
- type: "PUT",
- data: { id: this.id }
- }).then(result => {
- this.set("key", result.api_key.key);
- return this;
- });
+ @computed("key")
+ shortKey(key) {
+ return `${key.substring(0, 4)}...`;
+ },
+
+ @computed("description")
+ shortDescription(description) {
+ if (!description || description.length < 40) return description;
+ return `${description.substring(0, 40)}...`;
},
revoke() {
- return ajax(KEY_ENDPOINT, {
- type: "DELETE",
- data: { id: this.id }
- });
- }
-});
-
-ApiKey.reopenClass({
- create() {
- const result = this._super.apply(this, arguments);
- if (result.user) {
- result.user = AdminUser.create(result.user);
- }
- return result;
+ return ajax(`${this.basePath}/revoke`, {
+ type: "POST"
+ }).then(result => this.setProperties(result.api_key));
},
- find() {
- return ajax(KEYS_ENDPOINT).then(keys =>
- keys.map(key => ApiKey.create(key))
- );
+ undoRevoke() {
+ return ajax(`${this.basePath}/undo-revoke`, {
+ type: "POST"
+ }).then(result => this.setProperties(result.api_key));
},
- generateMasterKey() {
- return ajax(KEY_ENDPOINT, { type: "POST" }).then(result =>
- ApiKey.create(result.api_key)
- );
+ createProperties() {
+ return this.getProperties("description", "username");
+ },
+
+ @computed()
+ basePath() {
+ return this.store
+ .adapterFor("api-key")
+ .pathFor(this.store, "api-key", this.id);
}
});
diff --git a/app/assets/javascripts/admin/routes/admin-api-keys-index.js.es6 b/app/assets/javascripts/admin/routes/admin-api-keys-index.js.es6
new file mode 100644
index 0000000000..25f6df3e4a
--- /dev/null
+++ b/app/assets/javascripts/admin/routes/admin-api-keys-index.js.es6
@@ -0,0 +1,7 @@
+import Route from "@ember/routing/route";
+
+export default Route.extend({
+ model() {
+ return this.store.findAll("api-key");
+ }
+});
diff --git a/app/assets/javascripts/admin/routes/admin-api-keys-new.es6 b/app/assets/javascripts/admin/routes/admin-api-keys-new.es6
new file mode 100644
index 0000000000..3969615c9f
--- /dev/null
+++ b/app/assets/javascripts/admin/routes/admin-api-keys-new.es6
@@ -0,0 +1,7 @@
+import Route from "@ember/routing/route";
+
+export default Route.extend({
+ model() {
+ return this.store.createRecord("api-key");
+ }
+});
diff --git a/app/assets/javascripts/admin/routes/admin-api-keys-show.js.es6 b/app/assets/javascripts/admin/routes/admin-api-keys-show.js.es6
new file mode 100644
index 0000000000..b21f2cd021
--- /dev/null
+++ b/app/assets/javascripts/admin/routes/admin-api-keys-show.js.es6
@@ -0,0 +1,5 @@
+export default Ember.Route.extend({
+ model(params) {
+ return this.store.find("api-key", params.api_key_id);
+ }
+});
diff --git a/app/assets/javascripts/admin/routes/admin-api-keys.js.es6 b/app/assets/javascripts/admin/routes/admin-api-keys.js.es6
index 1826277567..1b3f3067fc 100644
--- a/app/assets/javascripts/admin/routes/admin-api-keys.js.es6
+++ b/app/assets/javascripts/admin/routes/admin-api-keys.js.es6
@@ -1,8 +1,13 @@
import Route from "@ember/routing/route";
-import ApiKey from "admin/models/api-key";
export default Route.extend({
- model() {
- return ApiKey.find();
+ actions: {
+ show(apiKey) {
+ this.transitionTo("adminApiKeys.show", apiKey.id);
+ },
+
+ new() {
+ this.transitionTo("adminApiKeys.new");
+ }
}
});
diff --git a/app/assets/javascripts/admin/routes/admin-route-map.js.es6 b/app/assets/javascripts/admin/routes/admin-route-map.js.es6
index 478a851d86..f90b66f97a 100644
--- a/app/assets/javascripts/admin/routes/admin-route-map.js.es6
+++ b/app/assets/javascripts/admin/routes/admin-route-map.js.es6
@@ -101,7 +101,14 @@ export default function() {
);
this.route("adminApi", { path: "/api", resetNamespace: true }, function() {
- this.route("adminApiKeys", { path: "/keys", resetNamespace: true });
+ this.route(
+ "adminApiKeys",
+ { path: "/keys", resetNamespace: true },
+ function() {
+ this.route("show", { path: "/:api_key_id" });
+ this.route("new", { path: "/new" });
+ }
+ );
this.route(
"adminWebHooks",
diff --git a/app/assets/javascripts/admin/templates/api-keys.hbs b/app/assets/javascripts/admin/templates/api-keys-index.hbs
similarity index 55%
rename from app/assets/javascripts/admin/templates/api-keys.hbs
rename to app/assets/javascripts/admin/templates/api-keys-index.hbs
index a1254f3042..19999029ea 100644
--- a/app/assets/javascripts/admin/templates/api-keys.hbs
+++ b/app/assets/javascripts/admin/templates/api-keys-index.hbs
@@ -1,7 +1,14 @@
+{{d-button
+ class="btn-primary"
+ action=(route-action "new")
+ icon="plus"
+ label="admin.api.new_key"}}
+
{{#if model}}
| {{i18n "admin.api.key"}} |
+ {{i18n "admin.api.description"}} |
{{i18n "admin.api.user"}} |
{{i18n "admin.api.created"}} |
{{i18n "admin.api.last_used"}} |
@@ -9,8 +16,14 @@
{{#each model as |k|}}
-
- | {{k.key}} |
+
+ |
+ {{#if k.revoked_at}}{{d-icon 'times-circle'}}{{/if}}
+ {{k.shortKey}}
+ |
+
+ {{k.shortDescription}}
+ |
{{i18n 'admin.api.user'}}
{{#if k.user}}
@@ -34,17 +47,21 @@
{{/if}}
|
- {{d-button
- class="btn-default"
- action=(action "regenerateKey")
- actionParam=k icon="undo"
- label="admin.api.regenerate"}}
- {{d-button
- class="btn-default"
- action=(action "revokeKey")
- actionParam=k
- icon="times"
- label="admin.api.revoke"}}
+ {{d-button action=(route-action "show" k) icon="far-eye" title="admin.api.show_details"}}
+ {{#if k.revoked_at}}
+ {{d-button
+ action=(action "undoRevokeKey")
+ actionParam=k icon="undo"
+ title="admin.api.undo-revoke"}}
+ {{else}}
+ {{d-button
+ class="btn-danger"
+ action=(action "revokeKey")
+ actionParam=k
+ icon="times"
+ title="admin.api.revoke"}}
+ {{/if}}
+
|
{{/each}}
@@ -52,11 +69,4 @@
{{else}}
{{i18n "admin.api.none"}}
-{{/if}}
-
-{{#unless hasMasterKey}}
- {{d-button
- class="btn-primary"
- action=(action "generateMasterKey")
- icon="key"}}
-{{/unless}}
+{{/if}}
\ No newline at end of file
diff --git a/app/assets/javascripts/admin/templates/api-keys-new.hbs b/app/assets/javascripts/admin/templates/api-keys-new.hbs
new file mode 100644
index 0000000000..15f25b1ce1
--- /dev/null
+++ b/app/assets/javascripts/admin/templates/api-keys-new.hbs
@@ -0,0 +1,27 @@
+{{#link-to 'adminApiKeys.index' class="go-back"}}
+ {{d-icon 'arrow-left'}}
+ {{i18n 'admin.api.all_api_keys'}}
+{{/link-to}}
+
+
+ {{#admin-form-row label="admin.api.description"}}
+ {{input value=model.description maxlength="255" placeholder=(i18n "admin.api.description_placeholder")}}
+ {{/admin-form-row}}
+
+ {{#admin-form-row label="admin.api.user_mode"}}
+ {{combo-box content=userModes value=userMode onSelect=(action "changeUserMode")}}
+ {{/admin-form-row}}
+
+ {{#if showUserSelector}}
+ {{#admin-form-row label="admin.api.user"}}
+ {{user-selector single="true"
+ usernames=model.username
+ placeholderKey="admin.api.user_placeholder"
+ }}
+ {{/admin-form-row}}
+ {{/if}}
+
+ {{#admin-form-row}}
+ {{d-button icon="check" label="admin.api.save" action=(action "save") class="btn-primary" disabled=saveDisabled}}
+ {{/admin-form-row}}
+
\ No newline at end of file
diff --git a/app/assets/javascripts/admin/templates/api-keys-show.hbs b/app/assets/javascripts/admin/templates/api-keys-show.hbs
new file mode 100644
index 0000000000..a742cf994a
--- /dev/null
+++ b/app/assets/javascripts/admin/templates/api-keys-show.hbs
@@ -0,0 +1,80 @@
+{{#link-to 'adminApiKeys.index' class="go-back"}}
+ {{d-icon 'arrow-left'}}
+ {{i18n 'admin.api.all_api_keys'}}
+{{/link-to}}
+
+
+ {{#admin-form-row label="admin.api.key"}}
+ {{#if model.revoked_at}}{{d-icon 'times-circle'}}{{/if}}
+ {{model.key}}
+ {{/admin-form-row}}
+
+ {{#admin-form-row label="admin.api.description"}}
+ {{#if editingDescription}}
+ {{input value=buffered.description maxlength="255" placeholder=(i18n "admin.api.description_placeholder")}}
+ {{else}}
+
{{if model.description model.description (i18n "admin.api.no_description")}}
+ {{/if}}
+
+
+ {{#if editingDescription}}
+ {{d-button class="ok" action=(action "saveDescription") icon="check"}}
+ {{d-button class="cancel" action=(action "editDescription") icon="times"}}
+ {{else}}
+ {{d-button class="btn-default" action=(action "editDescription") icon="pencil-alt"}}
+ {{/if}}
+
+ {{/admin-form-row}}
+
+ {{#admin-form-row label="admin.api.user"}}
+ {{#if model.user}}
+ {{#link-to "adminUser" model.user}}
+ {{avatar model.user imageSize="small"}} {{model.user.username}}
+ {{/link-to}}
+ {{else}}
+ {{i18n "admin.api.all_users"}}
+ {{/if}}
+ {{/admin-form-row}}
+
+ {{#admin-form-row label="admin.api.created"}}
+ {{format-date model.created_at leaveAgo="true"}}
+ {{/admin-form-row}}
+
+ {{#admin-form-row label="admin.api.updated"}}
+ {{format-date model.updated_at leaveAgo="true"}}
+ {{/admin-form-row}}
+
+ {{#admin-form-row label="admin.api.last_used"}}
+ {{#if k.last_used_at}}
+ {{format-date k.last_used_at leaveAgo="true"}}
+ {{else}}
+ {{i18n "admin.api.never_used"}}
+ {{/if}}
+ {{/admin-form-row}}
+
+ {{#admin-form-row label="admin.api.revoked"}}
+ {{#if model.revoked_at}}
+ {{format-date model.revoked_at leaveAgo="true"}}
+ {{/if}}
+
+ {{#if model.revoked_at}}
+ {{d-button
+ action=(action "undoRevokeKey")
+ actionParam=model icon="undo"
+ label="admin.api.undo_revoke"}}
+ {{d-button
+ action=(action "deleteKey")
+ actionParam=model icon="trash"
+ label="admin.api.delete"
+ class="btn-danger"}}
+ {{else}}
+ {{d-button
+ class="btn-danger"
+ action=(action "revokeKey")
+ actionParam=model
+ icon="times"
+ label="admin.api.revoke"}}
+ {{/if}}
+
+ {{/admin-form-row}}
+
\ No newline at end of file
diff --git a/app/assets/javascripts/admin/templates/user-index.hbs b/app/assets/javascripts/admin/templates/user-index.hbs
index 6166eb1b9a..7eb9504fb5 100644
--- a/app/assets/javascripts/admin/templates/user-index.hbs
+++ b/app/assets/javascripts/admin/templates/user-index.hbs
@@ -292,33 +292,14 @@
{{#if currentUser.admin}}
-
{{i18n "admin.api.key"}}
- {{#if model.api_key}}
-
- {{model.api_key.key}}
- {{d-button
- class="btn-default"
- action=(action "regenerateApiKey")
- icon="undo"
- label="admin.api.regenerate"}}
- {{d-button
- class="btn-default"
- action=(action "revokeApiKey")
- icon="times"
- label="admin.api.revoke"}}
-
- {{else}}
-
- —
-
-
- {{d-button
- class="btn-default"
- action=(action "generateApiKey")
- icon="key"
- label="admin.api.generate"}}
-
- {{/if}}
+
{{i18n "admin.api.active_keys"}}
+
+ {{model.api_key_count}}
+
+
+ {{d-button href="/admin/api/keys" label="admin.api.manage_keys"}}
+
+
{{/if}}
diff --git a/app/assets/stylesheets/common/admin/api.scss b/app/assets/stylesheets/common/admin/api.scss
index e2a921d585..ece2277a4f 100644
--- a/app/assets/stylesheets/common/admin/api.scss
+++ b/app/assets/stylesheets/common/admin/api.scss
@@ -73,18 +73,28 @@ table.web-hooks.grid {
table.api-keys {
margin-bottom: 0.25em;
- td.key {
- font-size: $font-down-1;
+
+ .key-controls {
+ text-align: right;
}
+
+ tr.revoked {
+ color: $primary-medium;
+ }
+
@include breakpoint(tablet) {
tr {
grid-template-columns: 0.25fr 1fr 1fr;
}
td.key {
- font-size: $font-0;
- word-wrap: break-word;
grid-row: 1;
grid-column-start: 1;
+ grid-column-end: 1;
+ max-width: 100%;
+ }
+ td.key-description {
+ grid-row: 1;
+ grid-column-start: 2;
grid-column-end: -1;
max-width: 100%;
}
@@ -138,6 +148,47 @@ table.api-keys {
}
}
+.admin-api-keys {
+ h2 {
+ margin-bottom: 10px;
+ }
+ .api-key {
+ padding: 10px;
+ margin-bottom: 10px;
+ border-bottom: 1px solid $primary-low;
+ .form-element,
+ .form-element-desc {
+ float: left;
+ padding: 0.5em 0;
+ &.input-area {
+ width: 75%;
+ .value-list,
+ .select-kit,
+ input[type="text"] {
+ width: 50%;
+ margin: 0;
+ }
+ .ac-wrap {
+ width: 50% !important;
+ }
+ }
+ &.label-area {
+ width: 25%;
+ label {
+ margin: 0.5em 1em 0 0;
+ text-align: right;
+ font-weight: bold;
+ }
+ }
+ }
+ .controls {
+ float: right;
+ text-align: left;
+ width: 50%;
+ }
+ }
+}
+
// Webhook
.web-hook-container {
.tip.good:empty {
diff --git a/app/controllers/admin/api_controller.rb b/app/controllers/admin/api_controller.rb
index 1af4a40d0b..42ec4035eb 100644
--- a/app/controllers/admin/api_controller.rb
+++ b/app/controllers/admin/api_controller.rb
@@ -1,16 +1,67 @@
# frozen_string_literal: true
class Admin::ApiController < Admin::AdminController
+ # Note: in the REST API, ApiKeys are referred to simply as "key"
+ # If we used "api_key", then our user provider would try to use the value for authentication
def index
- render_serialized(ApiKey.where(hidden: false).to_a, ApiKeySerializer)
+ keys = ApiKey.where(hidden: false)
+
+ # Put active keys first
+ # Sort active keys by created_at, sort revoked keys by revoked_at
+ keys = keys.order(<<~SQL)
+ CASE WHEN revoked_at IS NULL THEN 0 ELSE 1 END,
+ COALESCE(revoked_at, created_at) DESC
+ SQL
+
+ render_serialized(keys.to_a, ApiKeySerializer, root: 'keys')
end
- def regenerate_key
+ def show
+ api_key = ApiKey.find_by!(id: params[:id])
+ render_serialized(api_key, ApiKeySerializer, root: 'key')
+ end
+
+ def update
+ api_key = ApiKey.find_by!(id: params[:id])
+ ApiKey.transaction do
+ api_key.update!(update_params)
+ log_api_key(api_key, UserHistory.actions[:api_key_update], changes: api_key.saved_changes)
+ end
+ render_serialized(api_key, ApiKeySerializer, root: 'key')
+ end
+
+ def destroy
+ api_key = ApiKey.find_by!(id: params[:id])
+ ApiKey.transaction do
+ api_key.destroy
+ log_api_key(api_key, UserHistory.actions[:api_key_destroy])
+ end
+ render json: success_json
+ end
+
+ def create
+ api_key = ApiKey.new(update_params)
+ ApiKey.transaction do
+ api_key.created_by = current_user
+ if username = params.require(:key).permit(:username)[:username].presence
+ api_key.user = User.find_by_username(username)
+ raise Discourse::NotFound unless api_key.user
+ end
+ api_key.save!
+ log_api_key(api_key, UserHistory.actions[:api_key_create], changes: api_key.saved_changes)
+ end
+ render_serialized(api_key, ApiKeySerializer, root: 'key')
+ end
+
+ def undo_revoke_key
api_key = ApiKey.find_by(id: params[:id])
raise Discourse::NotFound if api_key.blank?
- api_key.regenerate!(current_user)
+ ApiKey.transaction do
+ api_key.update(revoked_at: nil)
+ log_api_key_restore(api_key)
+ end
render_serialized(api_key, ApiKeySerializer)
end
@@ -18,13 +69,32 @@ class Admin::ApiController < Admin::AdminController
api_key = ApiKey.find_by(id: params[:id])
raise Discourse::NotFound if api_key.blank?
- api_key.destroy
- render body: nil
- end
-
- def create_master_key
- api_key = ApiKey.create_master_key
+ ApiKey.transaction do
+ api_key.update(revoked_at: Time.zone.now)
+ log_api_key_revoke(api_key)
+ end
render_serialized(api_key, ApiKeySerializer)
end
+ private
+
+ def update_params
+ editable_fields = [:description]
+ permitted_params = params.permit(key: [*editable_fields])[:key]
+ raise Discourse::InvalidParameters unless permitted_params
+ permitted_params
+ end
+
+ def log_api_key(*args)
+ StaffActionLogger.new(current_user).log_api_key(*args)
+ end
+
+ def log_api_key_revoke(*args)
+ StaffActionLogger.new(current_user).log_api_key_revoke(*args)
+ end
+
+ def log_api_key_restore(*args)
+ StaffActionLogger.new(current_user).log_api_key_restore(*args)
+ end
+
end
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index 3261e8149a..bbfd7a0a54 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -19,8 +19,6 @@ class Admin::UsersController < Admin::AdminController
:add_group,
:remove_group,
:primary_group,
- :generate_api_key,
- :revoke_api_key,
:anonymize,
:reset_bounce_score,
:disable_second_factor,
@@ -102,7 +100,6 @@ class Admin::UsersController < Admin::AdminController
User.transaction do
@user.save!
- @user.revoke_api_key
user_history = StaffActionLogger.new(current_user).log_user_suspend(
@user,
@@ -179,16 +176,6 @@ class Admin::UsersController < Admin::AdminController
render body: nil
end
- def generate_api_key
- api_key = @user.generate_api_key(current_user)
- render_serialized(api_key, ApiKeySerializer)
- end
-
- def revoke_api_key
- @user.revoke_api_key
- render body: nil
- end
-
def grant_admin
AdminConfirmation.new(@user, current_user).create_confirmation
render json: success_json
diff --git a/app/jobs/scheduled/clean_up_unused_api_keys.rb b/app/jobs/scheduled/clean_up_unused_api_keys.rb
new file mode 100644
index 0000000000..4b9e756e63
--- /dev/null
+++ b/app/jobs/scheduled/clean_up_unused_api_keys.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+module Jobs
+
+ class CleanUpUnusedApiKeys < ::Jobs::Scheduled
+ every 1.day
+
+ def execute(args)
+ ApiKey.revoke_unused_keys!
+ end
+
+ end
+
+end
diff --git a/app/models/api_key.rb b/app/models/api_key.rb
index 6062025519..198d746b30 100644
--- a/app/models/api_key.rb
+++ b/app/models/api_key.rb
@@ -4,23 +4,44 @@ class ApiKey < ActiveRecord::Base
belongs_to :user
belongs_to :created_by, class_name: 'User'
- validates :user_id, uniqueness: true
+ scope :active, -> { where("revoked_at IS NULL") }
+ scope :revoked, -> { where("revoked_at IS NOT NULL") }
+
validates_presence_of :key
- def regenerate!(updated_by)
- self.key = SecureRandom.hex(32)
- self.created_by = updated_by
- save!
+ after_initialize :generate_key
+
+ def generate_key
+ self.key ||= SecureRandom.hex(32)
end
- def self.create_master_key
- api_key = ApiKey.find_by(user_id: nil, hidden: false)
- if api_key.blank?
- api_key = ApiKey.create(key: SecureRandom.hex(32), created_by: Discourse.system_user)
+ def truncated_key
+ self.key[0..3]
+ end
+
+ def self.last_used_epoch
+ SiteSetting.api_key_last_used_epoch.presence
+ end
+
+ def self.revoke_unused_keys!
+ return if SiteSetting.revoke_api_keys_days == 0 # Never expire keys
+ to_revoke = active.where("GREATEST(last_used_at, created_at, updated_at, :epoch) < :threshold",
+ epoch: last_used_epoch,
+ threshold: SiteSetting.revoke_api_keys_days.days.ago
+ )
+
+ to_revoke.find_each do |api_key|
+ ApiKey.transaction do
+ api_key.update!(revoked_at: Time.zone.now)
+
+ StaffActionLogger.new(Discourse.system_user).log_api_key(
+ api_key,
+ UserHistory.actions[:api_key_update],
+ changes: api_key.saved_changes,
+ context: I18n.t("staff_action_logs.api_key.automatic_revoked", count: SiteSetting.revoke_api_keys_days))
+ end
end
- api_key
end
-
end
# == Schema Information
diff --git a/app/models/user.rb b/app/models/user.rb
index adaf9ee9bb..ccd5928853 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -84,7 +84,7 @@ class User < ActiveRecord::Base
has_many :muted_user_records, class_name: 'MutedUser'
has_many :muted_users, through: :muted_user_records
- has_one :api_key, dependent: :destroy
+ has_many :api_keys, dependent: :destroy
has_many :push_subscriptions, dependent: :destroy
@@ -1028,19 +1028,6 @@ class User < ActiveRecord::Base
uploaded_avatar.present?
end
- def generate_api_key(created_by)
- if api_key.present?
- api_key.regenerate!(created_by)
- api_key
- else
- ApiKey.create!(user: self, key: SecureRandom.hex(32), created_by: created_by)
- end
- end
-
- def revoke_api_key
- ApiKey.where(user_id: self.id).delete_all
- end
-
def find_email
last_sent_email_address.present? && EmailValidator.email_regex =~ last_sent_email_address ? last_sent_email_address : email
end
diff --git a/app/models/user_history.rb b/app/models/user_history.rb
index 015d5bd15f..ec6fec4f11 100644
--- a/app/models/user_history.rb
+++ b/app/models/user_history.rb
@@ -97,7 +97,10 @@ class UserHistory < ActiveRecord::Base
web_hook_deactivate: 76,
change_theme_setting: 77,
disable_theme_component: 78,
- enable_theme_component: 79
+ enable_theme_component: 79,
+ api_key_create: 80,
+ api_key_update: 81,
+ api_key_destroy: 82,
)
end
@@ -171,7 +174,10 @@ class UserHistory < ActiveRecord::Base
:embeddable_host_destroy,
:change_theme_setting,
:disable_theme_component,
- :enable_theme_component
+ :enable_theme_component,
+ :api_key_create,
+ :api_key_update,
+ :api_key_destroy,
]
end
diff --git a/app/serializers/admin_detailed_user_serializer.rb b/app/serializers/admin_detailed_user_serializer.rb
index abe0d13773..3555522497 100644
--- a/app/serializers/admin_detailed_user_serializer.rb
+++ b/app/serializers/admin_detailed_user_serializer.rb
@@ -29,10 +29,10 @@ class AdminDetailedUserSerializer < AdminUserSerializer
:reset_bounce_score_after,
:can_view_action_logs,
:second_factor_enabled,
- :can_disable_second_factor
+ :can_disable_second_factor,
+ :api_key_count
has_one :approved_by, serializer: BasicUserSerializer, embed: :objects
- has_one :api_key, serializer: ApiKeySerializer, embed: :objects
has_one :suspended_by, serializer: BasicUserSerializer, embed: :objects
has_one :silenced_by, serializer: BasicUserSerializer, embed: :objects
has_one :tl3_requirements, serializer: TrustLevel3RequirementsSerializer, embed: :objects
@@ -118,4 +118,7 @@ class AdminDetailedUserSerializer < AdminUserSerializer
object.posts.count
end
+ def api_key_count
+ object.api_keys.active.count
+ end
end
diff --git a/app/serializers/api_key_serializer.rb b/app/serializers/api_key_serializer.rb
index 59f874be3e..c743b97d52 100644
--- a/app/serializers/api_key_serializer.rb
+++ b/app/serializers/api_key_serializer.rb
@@ -4,8 +4,11 @@ class ApiKeySerializer < ApplicationSerializer
attributes :id,
:key,
+ :description,
:last_used_at,
- :created_at
+ :created_at,
+ :updated_at,
+ :revoked_at
has_one :user, serializer: BasicUserSerializer, embed: :objects
diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb
index e564c6c790..f6264da3eb 100644
--- a/app/services/staff_action_logger.rb
+++ b/app/services/staff_action_logger.rb
@@ -663,6 +663,39 @@ class StaffActionLogger
))
end
+ def log_api_key(api_key, action, opts = {})
+ opts[:changes]&.delete("key") # Do not log the full key
+
+ history_params = params(opts).merge(
+ action: action,
+ subject: api_key.truncated_key
+ )
+
+ if opts[:changes]
+ old_values, new_values = get_changes(opts[:changes])
+ history_params[:previous_value] = old_values&.join(", ") unless opts[:changes].keys.include?("id")
+ history_params[:new_value] = new_values&.join(", ")
+ end
+
+ UserHistory.create!(history_params)
+ end
+
+ def log_api_key_revoke(api_key)
+ UserHistory.create!(params.merge(
+ subject: api_key.truncated_key,
+ action: UserHistory.actions[:api_key_update],
+ details: I18n.t("staff_action_logs.api_key.revoked")
+ ))
+ end
+
+ def log_api_key_restore(api_key)
+ UserHistory.create!(params.merge(
+ subject: api_key.truncated_key,
+ action: UserHistory.actions[:api_key_update],
+ details: I18n.t("staff_action_logs.api_key.restored")
+ ))
+ end
+
private
def get_changes(changes)
diff --git a/app/services/user_anonymizer.rb b/app/services/user_anonymizer.rb
index fbc560dec0..99c7dc6dd4 100644
--- a/app/services/user_anonymizer.rb
+++ b/app/services/user_anonymizer.rb
@@ -66,7 +66,7 @@ class UserAnonymizer
@user.user_associated_accounts.try(:destroy_all)
@user.instagram_user_info.try(:destroy)
@user.user_open_ids.find_each { |x| x.destroy }
- @user.api_key.try(:destroy)
+ @user.api_keys.find_each { |x| x.try(:destroy) }
@user.user_emails.secondary.destroy_all
@user_history = log_action
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index a431052d31..aeb0acdc58 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -3315,18 +3315,30 @@ en:
none: "There are no active API keys right now."
user: "User"
title: "API"
- key: "API Key"
+ key: "Key"
created: Created
+ updated: Updated
last_used: Last Used
never_used: (never)
generate: "Generate"
- regenerate: "Regenerate"
+ undo_revoke: "Undo Revoke"
revoke: "Revoke"
- confirm_regen: "Are you sure you want to replace that API Key with a new one?"
- confirm_revoke: "Are you sure you want to revoke that key?"
- info_html: "Your API key will allow you to create and update topics using JSON calls."
all_users: "All Users"
- note_html: "Keep this key secret, all users that have it may create arbitrary posts as any user."
+ active_keys: "Active API Keys"
+ manage_keys: Manage Keys
+ show_details: Details
+ description: Description
+ no_description: (no description)
+ all_api_keys: All API Keys
+ user_mode: User Level
+ impersonate_all_users: Impersonate any user
+ single_user: "Single User"
+ user_placeholder: Enter username
+ description_placeholder: What will this key be used for?
+ save: Save
+ new_key: New API Key
+ revoked: Revoked
+ delete: Permenantly Delete
web_hooks:
title: "Webhooks"
@@ -3918,6 +3930,9 @@ en:
change_theme_setting: "change theme setting"
disable_theme_component: "disable theme component"
enable_theme_component: "enable theme component"
+ api_key_create: "api key create"
+ api_key_update: "api key update"
+ api_key_destroy: "api key destroy"
screened_emails:
title: "Screened Emails"
description: "When someone tries to create a new account, the following email addresses will be checked and the registration will be blocked, or some other action performed."
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index b1619c47d4..fded4fd68d 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -2078,6 +2078,7 @@ en:
retain_web_hook_events_period_days: "Number of days to retain web hook event records."
retry_web_hook_events: "Automatically retry failed web hook events for 4 times. Time gaps between the retries are 1, 5, 25 and 125 minutes."
+ revoke_api_keys_days: "Number of days before an unused API key is automatically revoked (0 for never)"
allow_user_api_keys: "Allow generation of user API keys"
allow_user_api_key_scopes: "List of scopes allowed for user API keys"
@@ -4572,6 +4573,12 @@ en:
user_merged: "%{username} was merged into this account"
user_delete_self: "Deleted by self from %{url}"
webhook_deactivation_reason: "Your webhook has been automatically deactivated. We received multiple '%{status}' HTTP status failure responses."
+ api_key:
+ automatic_revoked:
+ one: "Automatically revoked, last activity more than %{count} day ago"
+ other: "Automatically revoked, last activity more than %{count} days ago"
+ revoked: Revoked
+ restored: Restored
reviewables:
priorities:
diff --git a/config/routes.rb b/config/routes.rb
index 81cc8d095e..8fb57fab5e 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -111,7 +111,6 @@ Discourse::Application.routes.draw do
put "revoke_admin", constraints: AdminConstraint.new
put "grant_admin", constraints: AdminConstraint.new
post "generate_api_key", constraints: AdminConstraint.new
- delete "revoke_api_key", constraints: AdminConstraint.new
put "revoke_moderation", constraints: AdminConstraint.new
put "grant_moderation", constraints: AdminConstraint.new
put "approve"
@@ -257,10 +256,12 @@ Discourse::Application.routes.draw do
resources :api, only: [:index], constraints: AdminConstraint.new do
collection do
- get "keys" => "api#index"
- post "key" => "api#create_master_key"
- put "key" => "api#regenerate_key"
- delete "key" => "api#revoke_key"
+ resources :keys, controller: 'api', only: [:index, :show, :update, :create, :destroy] do
+ member do
+ post "revoke" => "api#revoke_key"
+ post "undo-revoke" => "api#undo_revoke_key"
+ end
+ end
resources :web_hooks
get 'web_hook_events/:id' => 'web_hooks#list_events', as: :web_hook_events
diff --git a/config/site_settings.yml b/config/site_settings.yml
index a70e811b79..63f1aecf1c 100644
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@@ -2056,6 +2056,11 @@ api:
default: 30
retry_web_hook_events:
default: false
+ api_key_last_used_epoch:
+ default: "" # Value is added in a migration
+ hidden: true
+ revoke_api_keys_days:
+ default: 180
user_api:
allow_user_api_keys:
diff --git a/db/migrate/20191101113230_add_revoked_at_to_api_key.rb b/db/migrate/20191101113230_add_revoked_at_to_api_key.rb
new file mode 100644
index 0000000000..c6453ab159
--- /dev/null
+++ b/db/migrate/20191101113230_add_revoked_at_to_api_key.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+class AddRevokedAtToApiKey < ActiveRecord::Migration[5.2]
+ def up
+ add_column :api_keys, :revoked_at, :datetime
+ add_column :api_keys, :description, :text
+
+ execute "INSERT INTO site_settings(name, data_type, value, created_at, updated_at)
+ VALUES ('api_key_last_used_epoch', 1, now(), now(), now())"
+
+ remove_index :api_keys, :user_id # Remove unique index
+ add_index :api_keys, :user_id
+ end
+
+ def down
+ raise ActiveRecord::IrreversibleMigration
+ end
+end
diff --git a/lib/auth/default_current_user_provider.rb b/lib/auth/default_current_user_provider.rb
index 24ed4122fe..d7ffdbeb3e 100644
--- a/lib/auth/default_current_user_provider.rb
+++ b/lib/auth/default_current_user_provider.rb
@@ -281,7 +281,7 @@ class Auth::DefaultCurrentUserProvider
end
def lookup_api_user(api_key_value, request)
- if api_key = ApiKey.where(key: api_key_value).includes(:user).first
+ if api_key = ApiKey.active.where(key: api_key_value).includes(:user).first
api_username = header_api_key? ? @env[HEADER_API_USERNAME] : request[API_USERNAME]
if api_key.allowed_ips.present? && !api_key.allowed_ips.any? { |ip| ip.include?(request.ip) }
diff --git a/spec/components/auth/default_current_user_provider_spec.rb b/spec/components/auth/default_current_user_provider_spec.rb
index 14715096a8..616de7b4c0 100644
--- a/spec/components/auth/default_current_user_provider_spec.rb
+++ b/spec/components/auth/default_current_user_provider_spec.rb
@@ -65,6 +65,24 @@ describe Auth::DefaultCurrentUserProvider do
expect(key.last_used_at).to eq(nil)
end
+ it "raises for a revoked key" do
+ user = Fabricate(:user)
+ key = ApiKey.create!(key: "hello")
+ expect(
+ provider("/?api_key=hello&api_username=#{user.username.downcase}").current_user.id
+ ).to eq(user.id)
+
+ key.reload.update(revoked_at: Time.zone.now, last_used_at: nil)
+ expect(key.reload.last_used_at).to eq(nil)
+
+ expect {
+ provider("/?api_key=hello&api_username=#{user.username.downcase}").current_user
+ }.to raise_error(Discourse::InvalidAccess)
+
+ key.reload
+ expect(key.last_used_at).to eq(nil)
+ end
+
it "raises for a user with a mismatching ip" do
user = Fabricate(:user)
ApiKey.create!(key: "hello", user_id: user.id, created_by_id: -1, allowed_ips: ['10.0.0.0/24'])
diff --git a/spec/models/api_key_spec.rb b/spec/models/api_key_spec.rb
index fd6ac20303..aaaf719604 100644
--- a/spec/models/api_key_spec.rb
+++ b/spec/models/api_key_spec.rb
@@ -10,12 +10,59 @@ describe ApiKey do
it { is_expected.to belong_to :created_by }
it { is_expected.to validate_presence_of :key }
- it 'validates uniqueness of user_id' do
- Fabricate(:api_key, user: user)
- api_key = Fabricate.build(:api_key, user: user)
+ it 'generates a key when saving' do
+ key = ApiKey.new
+ key.save!
+ initial_key = key.key
+ expect(initial_key.length).to eq(64)
- expect(api_key.save).to eq(false)
- expect(api_key.errors).to include(:user_id)
+ # Does not overwrite key when saving again
+ key.description = "My description here"
+ key.save!
+ expect(key.reload.key).to eq(initial_key)
+ end
+
+ it "can calculate the epoch correctly" do
+ expect(ApiKey.last_used_epoch.to_datetime).to be_a(DateTime)
+
+ SiteSetting.api_key_last_used_epoch = ""
+ expect(ApiKey.last_used_epoch).to eq(nil)
+ end
+
+ it "can automatically revoke keys" do
+ now = Time.now
+
+ SiteSetting.api_key_last_used_epoch = now - 2.years
+ SiteSetting.revoke_api_keys_days = 180 # 6 months
+
+ freeze_time now - 1.year
+ never_used = Fabricate(:api_key)
+ used_previously = Fabricate(:api_key)
+ used_previously.update(last_used_at: Time.zone.now)
+ used_recently = Fabricate(:api_key)
+
+ freeze_time now - 3.months
+ used_recently.update(last_used_at: Time.zone.now)
+
+ freeze_time now
+ ApiKey.revoke_unused_keys!
+
+ [never_used, used_previously, used_recently].each(&:reload)
+ expect(never_used.revoked_at).to_not eq(nil)
+ expect(used_previously.revoked_at).to_not eq(nil)
+ expect(used_recently.revoked_at).to eq(nil)
+
+ # Restore them
+ [never_used, used_previously, used_recently].each { |a| a.update(revoked_at: nil) }
+
+ # Move the epoch to 1 month ago
+ SiteSetting.api_key_last_used_epoch = now - 1.month
+ ApiKey.revoke_unused_keys!
+
+ [never_used, used_previously, used_recently].each(&:reload)
+ expect(never_used.revoked_at).to eq(nil)
+ expect(used_previously.revoked_at).to eq(nil)
+ expect(used_recently.revoked_at).to eq(nil)
end
end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 567f94f6df..73eb8d7cac 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1135,56 +1135,6 @@ describe User do
end
end
- describe 'api keys' do
- fab!(:admin) { Fabricate(:admin) }
- fab!(:other_admin) { Fabricate(:admin) }
- fab!(:user) { Fabricate(:user) }
-
- describe '.generate_api_key' do
-
- it "generates an api key when none exists, and regenerates when it does" do
- expect(user.api_key).to be_blank
-
- # Generate a key
- api_key = user.generate_api_key(admin)
- expect(api_key.user).to eq(user)
- expect(api_key.key).to be_present
- expect(api_key.created_by).to eq(admin)
-
- user.reload
- expect(user.api_key).to eq(api_key)
-
- # Regenerate a key. Keeps the same record, updates the key
- new_key = user.generate_api_key(other_admin)
- expect(new_key.id).to eq(api_key.id)
- expect(new_key.key).to_not eq(api_key.key)
- expect(new_key.created_by).to eq(other_admin)
- end
-
- end
-
- describe '.revoke_api_key' do
-
- it "revokes an api key when exists" do
- expect(user.api_key).to be_blank
-
- # Revoke nothing does nothing
- user.revoke_api_key
- user.reload
- expect(user.api_key).to be_blank
-
- # When a key is present it is removed
- user.generate_api_key(admin)
- user.reload
- user.revoke_api_key
- user.reload
- expect(user.api_key).to be_blank
- end
-
- end
-
- end
-
describe "posted too much in topic" do
let!(:user) { Fabricate(:user, trust_level: TrustLevel[0]) }
let!(:topic) { Fabricate(:post).topic }
diff --git a/spec/requests/admin/api_controller_spec.rb b/spec/requests/admin/api_controller_spec.rb
index 4a20a47451..217ef19dc5 100644
--- a/spec/requests/admin/api_controller_spec.rb
+++ b/spec/requests/admin/api_controller_spec.rb
@@ -10,6 +10,9 @@ describe Admin::ApiController do
fab!(:admin) { Fabricate(:admin) }
+ fab!(:key1) { Fabricate(:api_key, description: "my key") }
+ fab!(:key2) { Fabricate(:api_key, user: admin) }
+
context "as an admin" do
before do
sign_in(admin)
@@ -19,60 +22,159 @@ describe Admin::ApiController do
it "succeeds" do
get "/admin/api/keys.json"
expect(response.status).to eq(200)
+ expect(JSON.parse(response.body)["keys"].length).to eq(2)
end
end
- describe '#regenerate_key' do
- fab!(:api_key) { Fabricate(:api_key) }
-
- it "returns 404 when there is no key" do
- put "/admin/api/key.json", params: { id: 1234 }
- expect(response.status).to eq(404)
- end
-
- it "delegates to the api key's `regenerate!` method" do
- prev_value = api_key.key
- put "/admin/api/key.json", params: { id: api_key.id }
+ describe '#show' do
+ it "succeeds" do
+ get "/admin/api/keys/#{key1.id}.json"
expect(response.status).to eq(200)
-
- api_key.reload
- expect(api_key.key).not_to eq(prev_value)
- expect(api_key.created_by.id).to eq(admin.id)
+ data = JSON.parse(response.body)["key"]
+ expect(data["id"]).to eq(key1.id)
+ expect(data["key"]).to eq(key1.key)
+ expect(data["description"]).to eq("my key")
end
end
- describe '#revoke_key' do
- fab!(:api_key) { Fabricate(:api_key) }
+ describe '#update' do
+ it "allows updating the description" do
+ original_key = key1.key
- it "returns 404 when there is no key" do
- delete "/admin/api/key.json", params: { id: 1234 }
- expect(response.status).to eq(404)
+ put "/admin/api/keys/#{key1.id}.json", params: {
+ key: {
+ description: "my new description",
+ key: "overridekey"
+ }
+ }
+ expect(response.status).to eq(200)
+
+ key1.reload
+ expect(key1.description).to eq("my new description")
+ expect(key1.key).to eq(original_key)
+
+ expect(UserHistory.last.action).to eq(UserHistory.actions[:api_key_update])
+ expect(UserHistory.last.subject).to eq(key1.truncated_key)
end
- it "delegates to the api key's `regenerate!` method" do
- delete "/admin/api/key.json", params: { id: api_key.id }
+ it "returns 400 for invalid payloads" do
+ put "/admin/api/keys/#{key1.id}.json", params: {
+ key: "string not a hash"
+ }
+ expect(response.status).to eq(400)
+
+ put "/admin/api/keys/#{key1.id}.json", params: {}
+ expect(response.status).to eq(400)
+ end
+ end
+
+ describe "#destroy" do
+ it "works" do
+ expect(ApiKey.exists?(key1.id)).to eq(true)
+
+ delete "/admin/api/keys/#{key1.id}.json"
+
expect(response.status).to eq(200)
- expect(ApiKey.where(key: api_key.key).count).to eq(0)
+ expect(ApiKey.exists?(key1.id)).to eq(false)
+
+ expect(UserHistory.last.action).to eq(UserHistory.actions[:api_key_destroy])
+ expect(UserHistory.last.subject).to eq(key1.truncated_key)
+ end
+ end
+
+ describe "#create" do
+ it "can create a master key" do
+ post "/admin/api/keys.json", params: {
+ key: {
+ description: "master key description"
+ }
+ }
+ expect(response.status).to eq(200)
+
+ data = JSON.parse(response.body)
+
+ expect(data['key']['description']).to eq("master key description")
+ expect(data['key']['user']).to eq(nil)
+ expect(data['key']['key']).to_not eq(nil)
+ expect(data['key']['last_used_at']).to eq(nil)
+
+ key = ApiKey.find(data['key']['id'])
+ expect(key.description).to eq("master key description")
+ expect(key.user).to eq(nil)
+
+ expect(UserHistory.last.action).to eq(UserHistory.actions[:api_key_create])
+ expect(UserHistory.last.subject).to eq(key.truncated_key)
+ end
+
+ it "can create a user-specific key" do
+ user = Fabricate(:user)
+ post "/admin/api/keys.json", params: {
+ key: {
+ description: "restricted key description",
+ username: user.username
+ }
+ }
+ expect(response.status).to eq(200)
+
+ data = JSON.parse(response.body)
+
+ expect(data['key']['description']).to eq("restricted key description")
+ expect(data['key']['user']['username']).to eq(user.username)
+ expect(data['key']['key']).to_not eq(nil)
+ expect(data['key']['last_used_at']).to eq(nil)
+
+ key = ApiKey.find(data['key']['id'])
+ expect(key.description).to eq("restricted key description")
+ expect(key.user.id).to eq(user.id)
+
+ expect(UserHistory.last.action).to eq(UserHistory.actions[:api_key_create])
+ expect(UserHistory.last.subject).to eq(key.truncated_key)
+ end
+ end
+
+ describe "#revoke and #undo_revoke" do
+ it "works correctly" do
+ post "/admin/api/keys/#{key1.id}/revoke.json"
+ expect(response.status).to eq 200
+
+ key1.reload
+ expect(key1.revoked_at).to_not eq(nil)
+ expect(UserHistory.last.action).to eq(UserHistory.actions[:api_key_update])
+ expect(UserHistory.last.subject).to eq(key1.truncated_key)
+ expect(UserHistory.last.details).to eq(I18n.t("staff_action_logs.api_key.revoked"))
+
+ post "/admin/api/keys/#{key1.id}/undo-revoke.json"
+ expect(response.status).to eq 200
+
+ key1.reload
+ expect(key1.revoked_at).to eq(nil)
+ expect(UserHistory.last.action).to eq(UserHistory.actions[:api_key_update])
+ expect(UserHistory.last.subject).to eq(key1.truncated_key)
+ expect(UserHistory.last.details).to eq(I18n.t("staff_action_logs.api_key.restored"))
end
end
end
- describe '#create_master_key' do
- it "creates a record" do
- sign_in(admin)
- expect do
- post "/admin/api/key.json"
- end.to change(ApiKey, :count).by(1)
- expect(response.status).to eq(200)
- end
-
- it "doesn't allow moderators to create master keys" do
+ context "as a moderator" do
+ before do
sign_in(Fabricate(:moderator))
- expect do
- post "/admin/api/key.json"
- end.to change(ApiKey, :count).by(0)
- expect(response.status).to eq(404)
end
+ it "doesn't allow access" do
+ get "/admin/api/keys.json"
+ expect(response.status).to eq(404)
+
+ get "/admin/api/key/#{key1.id}.json"
+ expect(response.status).to eq(404)
+
+ post "/admin/api/keys.json", params: {
+ key: {
+ description: "master key description"
+ }
+ }
+ expect(response.status).to eq(404)
+
+ expect(ApiKey.count).to eq(2)
+ end
end
end
diff --git a/spec/requests/admin/users_controller_spec.rb b/spec/requests/admin/users_controller_spec.rb
index e6bf9831d2..724b3a0ad4 100644
--- a/spec/requests/admin/users_controller_spec.rb
+++ b/spec/requests/admin/users_controller_spec.rb
@@ -123,25 +123,6 @@ RSpec.describe Admin::UsersController do
end
end
- describe '#generate_api_key' do
- it 'calls generate_api_key' do
- post "/admin/users/#{user.id}/generate_api_key.json"
- expect(response.status).to eq(200)
- json = JSON.parse(response.body)
- expect(json["api_key"]["user"]["id"]).to eq(user.id)
- expect(json["api_key"]["key"]).to be_present
- end
- end
-
- describe '#revoke_api_key' do
- it 'calls revoke_api_key' do
- ApiKey.create!(user: user, key: SecureRandom.hex)
- delete "/admin/users/#{user.id}/revoke_api_key.json"
- expect(response.status).to eq(200)
- expect(ApiKey.where(user: user).count).to eq(0)
- end
- end
-
describe '#suspend' do
fab!(:post) { Fabricate(:post) }
let(:suspend_params) do
@@ -269,15 +250,26 @@ RSpec.describe Admin::UsersController do
expect(log.details).to match(/long reason/)
end
- it "also revokes any api keys" do
- Fabricate(:api_key, user: user)
- put "/admin/users/#{user.id}/suspend.json", params: suspend_params
+ it "also prevents use of any api keys" do
+ api_key = Fabricate(:api_key, user: user)
+ put "/posts/#{Fabricate(:post).id}/bookmark.json", params: {
+ bookmarked: "true",
+ api_key: api_key.key
+ }
expect(response.status).to eq(200)
- user.reload
+ put "/admin/users/#{user.id}/suspend.json", params: suspend_params
+ expect(response.status).to eq(200)
+
+ user.reload
expect(user).to be_suspended
- expect(ApiKey.where(user_id: user.id).count).to eq(0)
+
+ put "/posts/#{Fabricate(:post).id}/bookmark.json", params: {
+ bookmarked: "true",
+ api_key: api_key.key
+ }
+ expect(response.status).to eq(403)
end
end
diff --git a/spec/requests/embed_controller_spec.rb b/spec/requests/embed_controller_spec.rb
index 5f6abb61f4..6969279502 100644
--- a/spec/requests/embed_controller_spec.rb
+++ b/spec/requests/embed_controller_spec.rb
@@ -42,7 +42,7 @@ describe EmbedController do
context "with api key" do
- let(:api_key) { ApiKey.create_master_key }
+ let(:api_key) { Fabricate(:api_key) }
context "with valid embed url" do
let(:topic_embed) { Fabricate(:topic_embed, embed_url: embed_url) }
diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb
index a7e8c7d1a1..45e4327f3f 100644
--- a/spec/requests/posts_controller_spec.rb
+++ b/spec/requests/posts_controller_spec.rb
@@ -524,8 +524,8 @@ describe PostsController do
end
context "api" do
- let(:api_key) { user.generate_api_key(user) }
- let(:master_key) { ApiKey.create_master_key }
+ let(:api_key) { Fabricate(:api_key, user: user) }
+ let(:master_key) { Fabricate(:api_key, user: nil) }
# choosing an arbitrarily easy to mock trusted activity
it 'allows users with api key to bookmark posts' do
@@ -711,7 +711,7 @@ describe PostsController do
raw = "this is a test post 123 #{SecureRandom.hash}"
title = "this is a title #{SecureRandom.hash}"
- master_key = ApiKey.create_master_key.key
+ master_key = Fabricate(:api_key).key
post "/posts.json", params: {
api_username: user.username,
@@ -740,7 +740,7 @@ describe PostsController do
Jobs.run_immediately!
NotificationEmailer.enable
post_1 = Fabricate(:post)
- master_key = ApiKey.create_master_key.key
+ master_key = Fabricate(:api_key).key
post "/posts.json", params: {
api_username: user.username,
@@ -796,7 +796,7 @@ describe PostsController do
it 'will raise an error if specified category cannot be found' do
user = Fabricate(:admin)
- master_key = ApiKey.create_master_key.key
+ master_key = Fabricate(:api_key).key
post "/posts.json", params: {
api_username: user.username,
diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb
index 357ce90fbb..afbc542e3b 100644
--- a/spec/requests/topics_controller_spec.rb
+++ b/spec/requests/topics_controller_spec.rb
@@ -1759,7 +1759,7 @@ RSpec.describe TopicsController do
end
context 'and the user is not logged in' do
- let(:api_key) { topic.user.generate_api_key(topic.user) }
+ let(:api_key) { Fabricate(:api_key, user: topic.user) }
it 'redirects to the login page' do
get "/t/#{topic.slug}/#{topic.id}.json"
diff --git a/spec/requests/user_badges_controller_spec.rb b/spec/requests/user_badges_controller_spec.rb
index aa03d6f7d8..07613c8193 100644
--- a/spec/requests/user_badges_controller_spec.rb
+++ b/spec/requests/user_badges_controller_spec.rb
@@ -111,10 +111,10 @@ describe UserBadgesController do
end
it 'does not grant badges from regular api calls' do
- Fabricate(:api_key, user: user)
+ api_key = Fabricate(:api_key, user: user)
post "/user_badges.json", params: {
- badge_id: badge.id, username: user.username, api_key: user.api_key.key
+ badge_id: badge.id, username: user.username, api_key: api_key.key
}
expect(response.status).to eq(403)
diff --git a/spec/services/user_anonymizer_spec.rb b/spec/services/user_anonymizer_spec.rb
index 3f9deb0e69..d534d8ca16 100644
--- a/spec/services/user_anonymizer_spec.rb
+++ b/spec/services/user_anonymizer_spec.rb
@@ -218,7 +218,7 @@ describe UserAnonymizer do
ApiKey.create(user_id: user.id, key: "123123123")
expect { make_anonymous }.to change { ApiKey.count }.by(-1)
user.reload
- expect(user.api_key).to eq(nil)
+ expect(user.api_keys).to be_empty
end
context "executes job" do
diff --git a/test/javascripts/admin/models/admin-user-test.js.es6 b/test/javascripts/admin/models/admin-user-test.js.es6
deleted file mode 100644
index 729cd55d05..0000000000
--- a/test/javascripts/admin/models/admin-user-test.js.es6
+++ /dev/null
@@ -1,30 +0,0 @@
-import AdminUser from "admin/models/admin-user";
-import ApiKey from "admin/models/api-key";
-
-QUnit.module("model:admin-user");
-
-QUnit.test("generate key", function(assert) {
- assert.expect(2);
-
- var adminUser = AdminUser.create({ id: 333 });
- assert.ok(!adminUser.get("api_key"), "it has no api key by default");
- return adminUser.generateApiKey().then(function() {
- assert.present(adminUser.get("api_key"), "it has an api_key now");
- });
-});
-
-QUnit.test("revoke key", function(assert) {
- assert.expect(2);
-
- var apiKey = ApiKey.create({ id: 1234, key: "asdfasdf" }),
- adminUser = AdminUser.create({ id: 333, api_key: apiKey });
-
- assert.equal(
- adminUser.get("api_key"),
- apiKey,
- "it has the api key in the beginning"
- );
- return adminUser.revokeApiKey().then(function() {
- assert.blank(adminUser.get("api_key"), "it cleared the api_key");
- });
-});
diff --git a/test/javascripts/helpers/create-pretender.js.es6 b/test/javascripts/helpers/create-pretender.js.es6
index d05a79a491..74d93e38ff 100644
--- a/test/javascripts/helpers/create-pretender.js.es6
+++ b/test/javascripts/helpers/create-pretender.js.es6
@@ -611,8 +611,6 @@ export default function() {
});
});
- this.post("/admin/users/:user_id/generate_api_key", success);
- this.delete("/admin/users/:user_id/revoke_api_key", success);
this.delete("/admin/users/:user_id.json", () =>
response(200, { deleted: true })
);