Issue Details (XML | Word | Printable)

Key: GLASSFISH-15375
Type: Bug Bug
Status: Resolved Resolved
Resolution: Fixed
Priority: Major Major
Assignee: Jason Lee
Reporter: Anissa Lam
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
glassfish

Doing a POST to property of auth-realm doesn't remove previous property

Created: 28/Dec/10 09:17 PM   Updated: 29/Dec/10 02:22 PM   Resolved: 29/Dec/10 02:22 PM
Component/s: rest-interface
Affects Version/s: 3.1_b33
Fix Version/s: 3.1_ms08

Time Tracking:
Not Specified

Tags: 3_1-approved
Participants: Anissa Lam, Chris Kasso and Jason Lee


 Description  « Hide

This is what i saw when working on another bug related to auth-realm. I don't think this is a bug in REST, since this behavior only occurs for auth-realm. I am filing this in REST so you can confirm REST is ok and the bug is in security code. Please transfer to security if this is the case.

In every other case, eg. web container, this works fine.
But in auth-realm, it is 'adding' the new property specified instead of completely replace them.

Here is the endpoint:
"http://localhost:4848/management/domain/configs/config/server-config/security-service/auth-realm/file/property"
Here is the 'raw data' when doing the POST.

"[{"selected":false,"description":"","name":"BBB","value":"BBB-value"},{"name":"file","value":"$\{com.sun.aas.instanceRoot\}\/config\/keyfile"},{"name":"jaas-context","value":"fileRealm"}]"

Before the post, domain.xml has:

<auth-realm classname="com.sun.enterprise.security.auth.realm.file.FileRealm" name="file">
<property name="file" value="${com.sun.aas.instanceRoot}/config/keyfile"></property>
<property name="jaas-context" value="fileRealm"></property>
<property name="AAA" value="AAA-value"></property>
</auth-realm>

After the POST, domain.xml has:
<auth-realm classname="com.sun.enterprise.security.auth.realm.file.FileRealm" name="file">
<property name="file" value="${com.sun.aas.instanceRoot}/config/keyfile"></property>
<property name="jaas-context" value="fileRealm"></property>
<property name="AAA" value="AAA-value"></property>
<property name="BBB" value="BBB-value"></property>
</auth-realm>

I will expect the property "AAA" to be gone.

Here is the step to reproduce this in GUI:

  • go to Configs -> server-config -> Security -> Realms in the tree and click on 'file'.
  • add a property named "AAA" and value "AAA-value"
  • SAVE.

go to common task page and then back to this page, (or look at domain.xml), this shows the property is added correctly .

Now, remove property "AAA" and add another property named "BBB" with "BBB-value". Save.

You will see that both "AAA" and "BBB" exists.
There is no way to remove old property.



Jason Lee added a comment - 29/Dec/10 10:51 AM

The handling of these properties goes through the same code path as all of the other ../property/ operations. When POSTing, all existing properties are cleared by getting a list of the child properties, calculating the dotted name for the property, then calling set with a blank value for the property. The new properties are then added back via the same code path, but with actual values passed.

In this case, I see the properties ('AAA') listed to be cleared, but I'm not seeing the property actually being removed. A work-around, though, is to use DELETE ../property/AAA which I have verified does work. This requires, though, special handling on the console, which is undesirable.

Reassigning to the admin team to investigate the interaction of the set command with the auth-realms. From the hip, we're sending a list of properties to clear, and it might be that one property (say 'fileRealm') is required and can't be cleared, so the set command is aborting. If that's the case, either set needs to be made less eager to quit, or the REST layer needs to call set iteratively rather than passing every property in one call. That approach is less efficient, but will likely be more reliable. I'll wait for feedback from the admin team to see which approach should be taken, assuming my guess is correct.


Jason Lee added a comment - 29/Dec/10 12:20 PM

I'm going to take this back. As I've thought more on it, I think the best solution is to modify the behavior of the REST interface. I think my hunch that we're seeing some sort of constraint on the auth realm makes more sense (though I'd still like to know for sure). Modifying the code in the REST layer to delete one at a time is trivial, and I don't think any extra processing incurred as a result of the repeated calls to set will be noticeable by the end user.


Jason Lee added a comment - 29/Dec/10 12:22 PM

How bad is its impact? (Severity)

Moderate. Unwanted properties would require a very manual process by the end user with either asadmin or a text editor, which is a suboptimal user experience.

How often does it happen? Will many users see this problem? (Frequency)

Every time

How much effort is required to fix it? (Cost)

Very little.

What is the risk of fixing it and how will the risk be mitigated? (Risk)

There is little to no risk in fixing this.

Diff:

Index: admin/rest/src/main/java/org/glassfish/admin/rest/resources/PropertiesBagResource.java
===================================================================
— admin/rest/src/main/java/org/glassfish/admin/rest/resources/PropertiesBagResource.java (revision 44144)
+++ admin/rest/src/main/java/org/glassfish/admin/rest/resources/PropertiesBagResource.java (working copy)
@@ -207,9 +207,8 @@
protected void deleteExistingProperties() throws TransactionFailure {
HashMap<String, String> data = new HashMap<String, String>();
for (final Dom existingProp : parent.nodeElements(tagName)) { + data.clear(); data.put (((ConfigBean) existingProp).attribute("name"), ""); - }

  • if (!data.isEmpty()) { Util.applyChanges(data, uriInfo, habitat); }
    }

Chris Kasso added a comment - 29/Dec/10 01:10 PM

Approved for 3.1


Jason Lee added a comment - 29/Dec/10 02:22 PM

Fix committed (r44154)