Skip to content

Commit

Permalink
Merge pull request #3866 from sarkapalkovicova/failed_application_notify
Browse files Browse the repository at this point in the history
fix(registrar)!: notify admins if application auto-approval fails
  • Loading branch information
sarkapalkovicova authored Feb 15, 2023
2 parents 79d59da + adfeeca commit 8bbb340
Show file tree
Hide file tree
Showing 12 changed files with 93 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public static enum AppType { INITIAL, EXTENSION, EMBEDDED }
private String extSourceType;
private int extSourceLoa = 0; // 0 - by default
private User user;

private String autoApproveError;
private String createdBy;
private String createdAt;
private String modifiedBy;
Expand Down Expand Up @@ -147,6 +147,10 @@ public String getModifiedAt() {
return modifiedAt;
}

public String getAutoApproveError() {
return autoApproveError;
}

public void setCreatedBy(String createdBy) {
this.createdBy = createdBy;
}
Expand All @@ -163,6 +167,10 @@ public void setModifiedAt(String modifiedAt) {
this.modifiedAt = modifiedAt;
}

public void setAutoApproveError(String error) {
this.autoApproveError = error;
}

/**
* Return bean name as PerunBean does.
*
Expand All @@ -181,15 +189,15 @@ public String toString() {
", fedInfo='" + getFedInfo() + '\'' +
", type='" + getType().toString() + '\'' +
", state='" + getState().toString() + '\'' +
", autoApproveError='" + getAutoApproveError() + '\'' +
", extSourceName='" + getExtSourceName() + '\'' +
", extSourceType='" + getExtSourceType() + '\'' +
", extSourceLoa='" + getExtSourceLoa() + '\'' +
", user='" + getUser() + '\'' +
", created_at='" + getCreatedAt() + '\'' +
", created_by='" + getCreatedBy() + '\'' +
", modified_at='" + getModifiedAt() + '\'' +
", modified_by='" + getModifiedBy() + '\'' +
']';
", modified_by='" + getModifiedBy() + '\'' + ']';
}

}
1 change: 1 addition & 0 deletions perun-base/src/test/resources/test-schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,7 @@ create table application (
state varchar, --state of application (new/verified/approved/rejected)
extSourceLoa integer, --level of assurance of user by external source
group_id integer, --identifier of group (groups.id) if application is for group
auto_approve_error varchar, --error that occurred during automatic approval
created_at timestamp default statement_timestamp() not null,
created_by varchar default user not null,
modified_at timestamp default statement_timestamp() not null,
Expand Down
20 changes: 17 additions & 3 deletions perun-cli/Perun/beans/Application.pm
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,14 @@ sub TO_JSON
$type = undef;
}

return { id => $id, vo => $vo, group => $group, user => $user, type => $type, state => $state };
my $autoApproveError;
if (defined($self->{_autoApproveError})) {
$autoApproveError = $self->{_autoApproveError};
} else {
$autoApproveError = undef;
}

return { id => $id, vo => $vo, group => $group, user => $user, type => $type, state => $state, autoApproveError => $autoApproveError };
}

sub getId
Expand Down Expand Up @@ -111,6 +118,12 @@ sub getState
return $self->{_state};
}

sub getAutoApproveError
{
my $self = shift;

return $self->{_autoApproveError};
}

sub getActor
{
Expand Down Expand Up @@ -145,11 +158,12 @@ sub getCommonArrayRepresentation {
my $self = shift;
return ($self->{_id}, $self->{_type}, $self->{_state}, $self->{_vo}->{id} . " / " . $self->{_vo}->{shortName},
((defined $self->{_group}) ? $self->{_group}->{id} . " / " . $self->{_group}->{name} : ""),
((defined $self->{_user}) ? $self->getUserDisplayName : $self->{_createdBy} . " / " . $self->{_extSourceName}));
((defined $self->{_user}) ? $self->getUserDisplayName : $self->{_createdBy} . " / " . $self->{_extSourceName}),
((defined $self->{_autoApproveError}) ? $self->{_autoApproveError} : ""));
}

sub getCommonArrayRepresentationHeading {
return ('ID', 'Type', 'State', 'Vo', 'Group', 'Submitted by');
return ('ID', 'Type', 'State', 'Vo', 'Group', 'Submitted by', 'Automatic approval error');
}

1;
1 change: 1 addition & 0 deletions perun-core/src/main/resources/postgresChangelog.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
3.2.11
ALTER TABLE vos DROP constraint vo_u;
ALTER TABLE vos ADD CONSTRAINT vo_u unique (short_name);
ALTER TABLE application ADD COLUMN auto_approve_error varchar;
UPDATE configurations set value='3.2.11' WHERE property='DATABASE VERSION';

3.2.10
Expand Down
1 change: 1 addition & 0 deletions perun-db/postgres.sql
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,7 @@ create table application (
state varchar, --state of application (new/verified/approved/rejected)
extSourceLoa integer, --level of assurance of user by external source
group_id integer, --identifier of group (groups.id) if application is for group
auto_approve_error varchar, --error that occurred during automatic approval
created_at timestamp default statement_timestamp() not null,
created_by varchar default user not null,
modified_at timestamp default statement_timestamp() not null,
Expand Down
1 change: 1 addition & 0 deletions perun-openapi/openapi.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1078,6 +1078,7 @@ components:
createdAt: { type: string }
modifiedBy: { type: string }
modifiedAt: { type: string }
autoApproveError: { type: string }

RichApplication:
allOf:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ public class MailManagerImpl implements MailManager {
private static final String FIELD_ACTOR = "{actor}";
private static final String FIELD_EXT_SOURCE = "{extSource}";
private static final String FIELD_CUSTOM_MESSAGE = "{customMessage}";
private static final String FIELD_AUTO_APPROVE_ERROR = "{autoApproveError}";
private static final String FIELD_FIRST_NAME = "{firstName}";
private static final String FIELD_LAST_NAME = "{lastName}";
private static final String FIELD_ERRORS = "{errors}";
Expand Down Expand Up @@ -1307,6 +1308,7 @@ private String buildInviteURL(Vo vo, Group group, String text) {
* {phone} - user phone submitted on application or stored in a system
*
* {customMessage} - message passed by the admin to mail (e.g. reason of application reject)
* {autoApproveError} - error that caused automatic approval failure
* {errors} - include errors, which occurred when processing registrar actions
* (e.g. login reservation errors passed to mail for VO admin)
*
Expand Down Expand Up @@ -1362,6 +1364,15 @@ private String substituteCommonStrings(Application app, List<ApplicationFormItem
}
}

// replace autoApproveError
if (mailText.contains(FIELD_AUTO_APPROVE_ERROR)) {
if (exceptions != null && !exceptions.isEmpty()) {
mailText = mailText.replace(FIELD_AUTO_APPROVE_ERROR, exceptions.get(0).getMessage());
} else {
mailText = mailText.replace(FIELD_AUTO_APPROVE_ERROR, EMPTY_STRING);
}
}

// replace {fromApp-*}
Matcher matcher = FROM_APP_PATTERN.matcher(mailText);
while (matcher.find()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1591,7 +1591,7 @@ public Application rejectApplication(PerunSession sess, int appId, String reason
}

// send mail
getMailManager().sendMessage(app, MailType.APP_REJECTED_USER, reason, null);
getMailManager().sendMessage(app, MailType.APP_REJECTED_USER, reason,null);

perun.getAuditer().log(sess, new ApplicationRejected(app));

Expand Down Expand Up @@ -1636,7 +1636,6 @@ private void deleteApplicationReservedLogins(PerunSession sess, Application app)

@Override
public Application approveApplication(PerunSession sess, int appId) throws PerunException {

synchronized(runningApproveApplication) {
if (runningApproveApplication.contains(appId)) {
throw new AlreadyProcessingException("Application approval is already processing.");
Expand Down Expand Up @@ -3776,29 +3775,29 @@ private void setApplicationState(PerunSession sess, int appId, AppState appState
/**
* Try to approve application if auto-approve is possible
*
* @param sess user who try to approves application
* @param app application to approve
* @param sess user who tries to approve application
* @param app application to be approved
* @throws InternalErrorException
*/
private void tryToAutoApproveApplication(PerunSession sess, Application app) throws PerunException {

ApplicationForm form;

if (app.getGroup() != null) {
// group application
form = getFormForGroup(app.getGroup());
} else {
// vo application
form = getFormForVo(app.getVo());
}

AppType type = app.getType();

if (AppType.INITIAL.equals(type) && !form.isAutomaticApproval()) return;
if (AppType.EXTENSION.equals(type) && !form.isAutomaticApprovalExtension()) return;
if (AppType.EMBEDDED.equals(type) && !form.isAutomaticApprovalEmbedded()) return;
if ((AppType.INITIAL.equals(type) && !form.isAutomaticApproval()) || (AppType.EXTENSION.equals(type) && !form.isAutomaticApprovalExtension()) || (AppType.EMBEDDED.equals(type) && !form.isAutomaticApprovalEmbedded())) {
return;
}

// do not auto-approve Group applications, if user is not member of VO
if (app.getGroup() != null && app.getVo() != null) {

try {
if (app.getUser() == null) {
LinkedHashMap<String, String> additionalAttributes = BeansUtils.stringToMapOfAttributes(app.getFedInfo());
Expand All @@ -3808,19 +3807,15 @@ private void tryToAutoApproveApplication(PerunSession sess, Application app) thr
membersManager.getMemberByUser(sess, app.getVo(), u);
} else {
// user not found or null, hence can't be member of VO -> do not approve.
setAutoApproveErrorToApplication(app, "This application is waiting for approval of the VO application, which must be approved by the VO manager first. After that, this application will be automatically approved.");
return;
}
} else {
// user known, but maybe not member of a vo
membersManager.getMemberByUser(sess, app.getVo(), app.getUser());
}
} catch (MemberNotExistsException ex) {
return;
} catch (UserNotExistsException ex) {
return;
} catch (UserExtSourceNotExistsException ex) {
return;
} catch (ExtSourceNotExistsException ex) {
} catch (MemberNotExistsException | UserNotExistsException | UserExtSourceNotExistsException | ExtSourceNotExistsException ex) {
setAutoApproveErrorToApplication(app, "This application is waiting for approval of the VO application, which must be approved by the VO manager first. After that, this application will be automatically approved.");
return;
}
}
Expand Down Expand Up @@ -3854,14 +3849,10 @@ private void tryToAutoApproveApplication(PerunSession sess, Application app) thr

}
} catch (Exception ex) {

ArrayList<Exception> list = new ArrayList<>();
list.add(ex);
getMailManager().sendMessage(app, MailType.APP_ERROR_VO_ADMIN, null, list);

setAutoApproveErrorToApplication(app, ex.getMessage());
getMailManager().sendMessage(app, MailType.APP_ERROR_VO_ADMIN, null, List.of(ex));
throw ex;
}

}

/**
Expand Down Expand Up @@ -4803,6 +4794,16 @@ private void fixDependencies(ApplicationForm form, Map<Integer, Integer> idsTran
}
}

/**
* Sets error that occurred during automatic approval of application.
*
* @param application application
* @param error error
*/
private void setAutoApproveErrorToApplication(Application application, String error) {
jdbc.update("UPDATE application SET auto_approve_error=? WHERE id=?", error, application.getId());
}

/**
* Returns number of embedded groups form items.
*/
Expand Down Expand Up @@ -4942,15 +4943,15 @@ private static ResultSetExtractor<Paginated<RichApplication>> getPaginatedApplic
}

// FIXME - we are retrieving GROUP name using only "short_name" so it's not same as getGroupById()
static final String APP_SELECT = "select a.id as id,a.vo_id as vo_id, a.group_id as group_id,a.apptype as apptype,a.fed_info as fed_info,a.state as state," +
"a.user_id as user_id,a.extsourcename as extsourcename, a.extsourcetype as extsourcetype, a.extsourceloa as extsourceloa, a.user_id as user_id, a.created_at as app_created_at, a.created_by as app_created_by, a.modified_at as app_modified_at, a.modified_by as app_modified_by, " +
static final String APP_SELECT = "select a.id as id, a.vo_id as vo_id, a.group_id as group_id,a.apptype as apptype,a.fed_info as fed_info,a.state as state," +
"a.user_id as user_id, a.auto_approve_error as auto_approve_error, a.extsourcename as extsourcename, a.extsourcetype as extsourcetype, a.extsourceloa as extsourceloa, a.user_id as user_id, a.created_at as app_created_at, a.created_by as app_created_by, a.modified_at as app_modified_at, a.modified_by as app_modified_by, " +
"v.name as vo_name, v.short_name as vo_short_name, v.created_by as vo_created_by, v.created_at as vo_created_at, v.created_by_uid as vo_created_by_uid, v.modified_by as vo_modified_by, " +
"v.modified_at as vo_modified_at, v.modified_by_uid as vo_modified_by_uid, g.name as group_name, g.dsc as group_description, g.created_by as group_created_by, g.created_at as group_created_at, g.modified_by as group_modified_by, g.created_by_uid as group_created_by_uid, g.modified_by_uid as group_modified_by_uid," +
"g.modified_at as group_modified_at, g.vo_id as group_vo_id, g.parent_group_id as group_parent_group_id, g.uu_id as group_uu_id, u.first_name as user_first_name, u.last_name as user_last_name, u.middle_name as user_middle_name, " +
"u.title_before as user_title_before, u.title_after as user_title_after, u.service_acc as user_service_acc, u.sponsored_acc as user_sponsored_acc , u.uu_id as user_uu_id from application a left outer join vos v on a.vo_id = v.id left outer join groups g on a.group_id = g.id left outer join users u on a.user_id = u.id";

static final String APP_SELECT_PAGE = "select a.id as id,a.vo_id as vo_id, a.group_id as group_id,a.apptype as apptype,a.fed_info as fed_info,a.state as state," +
"a.user_id as user_id,a.extsourcename as extsourcename, a.extsourcetype as extsourcetype, a.extsourceloa as extsourceloa, a.user_id as user_id, a.created_at as app_created_at, a.created_by as app_created_by, a.modified_at as app_modified_at, a.modified_by as app_modified_by, " +
static final String APP_SELECT_PAGE = "select a.id as id, a.vo_id as vo_id, a.group_id as group_id, a.apptype as apptype, a.fed_info as fed_info,a.state as state," +
"a.auto_approve_error as auto_approve_error, a.user_id as user_id,a.extsourcename as extsourcename, a.extsourcetype as extsourcetype, a.extsourceloa as extsourceloa, a.user_id as user_id, a.created_at as app_created_at, a.created_by as app_created_by, a.modified_at as app_modified_at, a.modified_by as app_modified_by, " +
"v.name as vo_name, v.short_name as vo_short_name, v.created_by as vo_created_by, v.created_at as vo_created_at, v.created_by_uid as vo_created_by_uid, v.modified_by as vo_modified_by, " +
"v.modified_at as vo_modified_at, v.modified_by_uid as vo_modified_by_uid, g.name as group_name, g.dsc as group_description, g.created_by as group_created_by, g.created_at as group_created_at, g.modified_by as group_modified_by, g.created_by_uid as group_created_by_uid, g.modified_by_uid as group_modified_by_uid," +
"g.modified_at as group_modified_at, g.vo_id as group_vo_id, g.parent_group_id as group_parent_group_id, g.uu_id as group_uu_id, u.first_name as user_first_name, u.last_name as user_last_name, u.middle_name as user_middle_name, " +
Expand Down Expand Up @@ -5020,6 +5021,7 @@ private static ResultSetExtractor<Paginated<RichApplication>> getPaginatedApplic
app.setCreatedBy(resultSet.getString("app_created_by"));
app.setModifiedAt(resultSet.getString("app_modified_at"));
app.setModifiedBy(resultSet.getString("app_modified_by"));
app.setAutoApproveError(resultSet.getString("auto_approve_error"));

return app;

Expand Down
4 changes: 2 additions & 2 deletions perun-utils/rpc-methods-javadoc-generator/parseRpcMethods.pl
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,11 @@
$objectExamples{"List&lt;RichGroup&gt;"} = $listPrepend . $objectExamples{"RichGroup"} . $listAppend;
$objectExamples{"List<RichGroup>"} = $objectExamples{"List&lt;RichGroup&gt;"};

$objectExamples{"Application"} = "{ \"id\" : 12 , \"vo\" : ". $objectExamples{"Vo"} . " , \"type\" : \"INITIAL\" , \"fedInfo\" : \"\" , \"state\" : \"NEW\" , \"extSourceName\" : \"PERUNPEOPLE\" , \"extSourceType\" : \"cz.metacentrum.perun.core.impl.ExtSourceSql\" , \"user\" : " . $objectExamples{"User"} . ", \"beanName\" : \"Application\" }";
$objectExamples{"Application"} = "{ \"id\" : 12 , \"vo\" : ". $objectExamples{"Vo"} . " , \"type\" : \"INITIAL\" , \"fedInfo\" : \"\" , \"state\" : \"NEW\" , \"autoApproveError\" : null , \"extSourceName\" : \"PERUNPEOPLE\" , \"extSourceType\" : \"cz.metacentrum.perun.core.impl.ExtSourceSql\" , \"user\" : " . $objectExamples{"User"} . ", \"beanName\" : \"Application\" }";
$objectExamples{"List&lt;Application&gt;"} = $listPrepend . $objectExamples{"Application"} . $listAppend;
$objectExamples{"List<Application>"} = $objectExamples{"List&lt;Application&gt;"};

$objectExamples{"RichApplication"} = "{ \"id\" : 12 , \"vo\" : ". $objectExamples{"Vo"} . " , \"type\" : \"INITIAL\" , \"fedInfo\" : \"\" , \"state\" : \"NEW\" , \"extSourceName\" : \"PERUNPEOPLE\" , \"extSourceType\" : \"cz.metacentrum.perun.core.impl.ExtSourceSql\" , \"user\" : " . $objectExamples{"User"} . ", \"beanName\" : \"RichApplication\", \"formData\" : " . $objectExamples{"List<ApplicationFormItemData>"} . " }";
$objectExamples{"RichApplication"} = "{ \"id\" : 12 , \"vo\" : ". $objectExamples{"Vo"} . " , \"type\" : \"INITIAL\" , \"fedInfo\" : \"\" , \"state\" : \"NEW\" , \"autoApproveError\" : null , \"extSourceName\" : \"PERUNPEOPLE\" , \"extSourceType\" : \"cz.metacentrum.perun.core.impl.ExtSourceSql\" , \"user\" : " . $objectExamples{"User"} . ", \"beanName\" : \"RichApplication\", \"formData\" : " . $objectExamples{"List<ApplicationFormItemData>"} . " }";
$objectExamples{"List&lt;RichApplication&gt;"} = $listPrepend . $objectExamples{"RichApplication"} . $listAppend;
$objectExamples{"List<RichApplication>"} = $objectExamples{"List&lt;RichApplication&gt;"};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,24 @@ public final native void setExtSourceName(String extSourceName) /*-{
this.extSourceName = extSourceName;
}-*/;

/**
* Get error that occurred during the automatic approval of the application.
*
* @return error
*/
public final native String getAutoApproveError() /*-{
return this.autoApproveError;
}-*/;

/**
* Set error that occurred during the automatic approval of the application.
*
* @param autoApproveError error
*/
public final native void setAutoApproveError(String autoApproveError) /*-{
this.autoApproveError = autoApproveError;
}-*/;

/**
* Get extSourceType
* @return extSourceType
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,7 @@ private Widget availableTagsTab() {
"<br/><strong>{mailFooter}</strong> - common mail footer defined by VO" +
"<br/><strong>{errors}</strong> - errors description, what happened while processing new application. Useful for VO administrators." +
"<br/><strong>{customMessage}</strong> - optional message passed by administrators when rejecting an application" +
"<br/><strong>{autoApproveError}</strong> - error that caused automatic approval failure" +
"<br/><strong>{fromApp-itemName}</strong> - value of a form item in user's application. You MUST specify the itemName, e.g. {fromApp-mail} will print value of item with short name 'mail' from user's application." +

"<p><strong><u>User related:</u></strong>" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ private Widget availableTagsTab() {
"<br/><strong>{mailFooter}</strong> - common mail footer defined by VO" +
"<br/><strong>{errors}</strong> - errors description, what happened while processing new application. Useful for VO administrators." +
"<br/><strong>{customMessage}</strong> - optional message passed by administrators when rejecting an application" +
"<br/><strong>{autoApproveError}</strong> - error that caused automatic approval failure" +
"<br/><strong>{fromApp-itemName}</strong> - value of a form item in user's application. You MUST specify the itemName, e.g. {fromApp-mail} will print value of item with short name 'mail' from user's application." +

"<p><strong><u>User related:</u></strong>" +
Expand Down

0 comments on commit 8bbb340

Please sign in to comment.